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

Bookmark search #2873

Merged
merged 11 commits into from Oct 31, 2018
Merged

Bookmark search #2873

merged 11 commits into from Oct 31, 2018

Conversation

johnhaddon
Copy link
Member

This adds the following :

  • A right click context menu to pinning icons. It can be customised completely via config files, but currently provides a menu item to bookmark the current node, and one to search for a bookmark to view. It can be customised by config files.
  • A Ctrl+B shortcut for all NodeSetEditors and also the GraphEditor. This initiates a bookmark search for the editor under the pointer. Bookmark search operates in the same manner as the search in the node creation tab menu.

findbookmark

This is based on suggestions made by Murray and Yuta during internal discussion of #2784. It also provides some groundwork for improving the implementation of #2729, particularly replacing the hardcoding and duplication of d97dd8f with a simple bolt-on config for all editors.

By default, Qt draws an additional focus box that we don't want.
This is customised via the new `nodeSetMenuSignal()`.
This allows custom extensions to add shortcuts for any editor. It also fixes GafferHQ#495.
This allows keyboard shortcuts registered by extensions to work even without a node selected.
This allows keyboard shortcuts to be processed still. Perhaps we should be deferring the enabled status updates to the sections themselves anyway?
@johnhaddon
Copy link
Member Author

Bookmark search operates in the same manner as the search in the node creation tab menu.

I realise this might be a misleading description, given that my screenshot only has nodes with default names. The structure of the menu actually matches the structure (nesting of boxes) of the node graph, and uses the names and not the types of the nodes. What is similar is that the same search code is used, and the same disambiguation mechanism is used when there are multiple matches at different points of the hierarchy.

Copy link
Contributor

@andrewkaufman andrewkaufman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! Just the one bug I noticed.

Also, would it be worth adding a shortcut for bookmarking a node? Its so slick to fetch the bookmarks now, but adding one is a but clunky by comparison... mabye Shift+B?

python/GafferUI/GraphBookmarksUI.py Show resolved Hide resolved
@johnhaddon
Copy link
Member Author

Also, would it be worth adding a shortcut for bookmarking a node?

I'm going to say no for now, both because creating the bookmarks is less common, and because in most of the editors we won't be providing any visual feedback as to what has happened. We can revisit based on wider feedback, and/or after Matti's #2729 has added the super-quick bookmarking for super-important nodes.

@andrewkaufman andrewkaufman merged commit 57c4598 into GafferHQ:master Oct 31, 2018
@johnhaddon johnhaddon deleted the bookmarkSearch branch November 2, 2018 14:41
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.

None yet

2 participants