Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Copy Current Filter" action #1620

Merged
merged 3 commits into from
Jan 28, 2025
Merged

Add "Copy Current Filter" action #1620

merged 3 commits into from
Jan 28, 2025

Conversation

bmatherly
Copy link
Member

As suggested here:
https://forum.shotcut.org/t/copy-single-filters/13909

This PR adds a new button next to the existing "Copy Filters" Button. This new button is called "Copy Selected Filter".
image

In the forum discussion, the conversation converged on the idea of adding multi-select to the filter list so that the user can choose what filters to copy. I am in support of this. However, I only implemented single filter copy in this pull request. The reasons that I did not implement multi-select are:

  1. The QML ListView Item does not offer multi-select. So I added a lot of extra logic to disable the built-in selection and to implement my own multi-select implementation on top of it. It was mostly working.
  2. After I implemented multiselect for the list view, I found that there are a lot of dynamic interactions between the QML filter list, filter controller, and filter dock. I kept adding more and more code to keep all three classes happy with the multi-select.

Eventually, I decided that the extra complexity and risk of regression is too great and I changed to this simpler single selection implementation.

@ddennedy
Copy link
Member

I am not a fan of the icon and adding a confusing alternative copy action next to the other on the tool bar. Perhaps the existing copy should have a drop down arrow to choose from alternatives:

  • Enabled (checked)
  • All
  • Selected

You must click the drop down arrow to access the alternatives (click icon chooses Enabled) unless you use the actions window or assign a shortcut. What do you think of that proposal?

Remove extra button and put copy options in a menu
@bmatherly
Copy link
Member Author

I agree with your comments. Unfortunately, the QML button Item does not have all the same features as a QButton and does not offer the dual function of an action and a submenu.

I submitted a variation that always shows the submenu with the three options.
image

I think that maybe it looks a little strange that they all have the same icon. Maybe I should just remove the icon.

@ddennedy
Copy link
Member

ddennedy commented Jan 13, 2025

Please remove the icons from the submenu. Do we need the “filters suffix?” It should appear in the “Filters >” group in the Actions window. The singular vs. plural might raise issues from translators, and the plural form of the tr method needs to supplied with a count parameter. Otherwise, I’m fine that it always opens the menu.

@bmatherly
Copy link
Member Author

Both good comments. This is done. You were right - the Shortcuts menu still makes sense because it says "Filters -> Copy All", etc.

@ddennedy ddennedy added this to the v25.02 milestone Jan 28, 2025
@ddennedy ddennedy merged commit aeb5ffb into master Jan 28, 2025
1 check passed
@ddennedy ddennedy deleted the filter_copy branch January 28, 2025 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants