-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Change documentsearch shortcut selector to searchable class #6237
Change documentsearch shortcut selector to searchable class #6237
Conversation
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
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.
Nice. Thank you!
👍
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.
I thought about this PR some more. To make document search available to non-JupyterLab applications more easily, this ILabShell
logic could be in its own JupyterFrontEndPlugin<void>
so that the main search plugin that adds the commands installs without that hard dependency.
To elaborate for @aschlaep - this would just be splitting this one piece of logic involving the labshell into its own plugin. A test of how this makes things more flexible would be adding document search to the plain notebook example at https://github.com/jupyterlab/jupyterlab/tree/master/examples/notebook. |
I moved the css class application to its own plugin and added documentsearch to the notebook example, with help from @jasongrout! |
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.
Very nice, thanks!
This addresses bullet 1 in #6217. When the active widget changes, the documentsearch extension checks if it is searchable and applies the
jp-mod-searchable
class if it doesn't have it already. Then theAccel F
and other shortcuts are scoped to only this class, so that they'll pass through to the browser when the focus is outside a searchable widget.@afshin @jasongrout