-
Notifications
You must be signed in to change notification settings - Fork 206
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
Crop Window Toggle #5518
Crop Window Toggle #5518
Conversation
f3b6342
to
34a7926
Compare
I addressed the comments above - despite it being a bit weird that two of the CI builds failed in tests, I think this is ready for another look. Those failures seem unrelated. |
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.
Sorry Eric, I spotted one more problem that I should have spotted the first time around - see inline comment. Otherwise LGTM - could you squash everything appropriately while adding the UndoScope and then we can merge?
Definitely. I've pushed bf248a4 to take care of one of them. The Windows one I've seen before too, but don't know the cause yet. |
34a7926
to
9272959
Compare
Squashed and ready for merging as long as my |
Previously we were only toggling non-exclusive tools off with keyboard shortcuts. This changes toggle behavior to activate the first tool in a toolbar when an exclusive tool is toggled off. This is initially motivated by the desire to be able to toggle off the CropWindowTool.
9272959
to
1e105c8
Compare
void toolsChildAdded( Gaffer::GraphComponent *child ); | ||
void toolPlugSet( Gaffer::Plug *plug ); | ||
|
||
using ToolPlugSetMap = std::unordered_map<Tool *, Gaffer::Signals::ScopedConnection>; |
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.
In the last push, I changed this to be keyed to Tool *
instead of the name of the tool. Though we don't expect the tool name to change, it is possible. Keying on Tool *
is more reliable.
This changes two related behaviors :
Alt+C
shortcut to toggle off the crop window tool as well as the crop window itself, for the plug used for the crop window tool.Checklist