-
Notifications
You must be signed in to change notification settings - Fork 40
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
SG-36193 Adds named filter support #171
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pscadding Looks good, but I do have a comment about refactoring how preset filters are handled. Let me know if it makes sense, or if you need any more clarification on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After more thought, the preset filters should remain as backend-filters (ideally the framework could handle this use case, but it needs a lot more work to do so).
Other than that, this is looking good, just a couple minor comments.
made condition easier to read. Co-authored-by: Stacey Oue <stacey.oue@autodesk.com>
….com/shotgunsoftware/tk-framework-qtwidgets into ticket/SG-36193_custom-named-filters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Adds support for creating more complex filters shown in the filter menu as a preset filter.
This mimics the web UI behaviour in part, although there is no ability to modify the filters from the UI, only toggle which one is used.
There is a separate PR for the panel app listed below and the changes here to the framework are implemented to allow support for the panel app to work. I don't believe any of the included changes would work against implementing this in other apps like loader2 or breakdown app 2, but there would be further work needed to support those apps such as:
I did do some work into getting it to work in the loader, and had implemented the state storing but I've removed it due to the loader implementation becoming more work on the loader side.
Panel app support is added here:
shotgunsoftware/tk-multi-shotgunpanel#97
I did look at making the preset buttons larger to match the other filter buttons but this became quite involved and in my opinion added unnecessary complexity for this particular use. From what I could tell I could either adjust the styling on all menu items, or I would need to create an manage a widget that gets applied to the menu so that I could style it individually.