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

Page_config attribute to handle keydown event at bubbling phase #15142

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

brichet
Copy link
Contributor

@brichet brichet commented Sep 21, 2023

EDITED AFTER MERGE

The code changed in this PR was experimental and has been removed in #14115, in this commit 5c6c7d7, to avoid application instability.

INITIAL DESCRIPTION

This PR adds a new attribute to the page_config.json file, to allow handling the keydown event at bubbling phase instead of capture phase.


page_config.json

{
  "bubbling_keydown": true
}

See the documentation to set it up.

References

See #14115 (comment) for context.

Draft PR until jupyterlab/lumino#635 is merged and released.

Code changes

When launching the application (dev_mode/index.js), the value of bubbling_keydown (boolean) is send to the lumino Application.
If the bubbling_keydown is false or undefined, the application starts as usual.

User-facing changes

None

Backwards-incompatible changes

None

cc. @fcollonval @krassowski @gabalafou

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval
Copy link
Member

Ok I did not think about using the pageconfig. It indeed works without the need for a dynamic settings change in Lumino. But is it convenient enough for testing?

@brichet
Copy link
Contributor Author

brichet commented Sep 21, 2023

Ok I did not think about using the pageconfig. It indeed works without the need for a dynamic settings change in Lumino. But is it convenient enough for testing?

Yeah, I did not think about a dynamic flag 😄
This PR is probably useless since the dynamic flag is working. Except if we want long term testing, than it can be useful to avoid setting up the flag each time.

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @brichet

@fcollonval
Copy link
Member

I'm moving forward with this to be able to get it in an alpha release

@fcollonval
Copy link
Member

CI failures are due to flakiness

@fcollonval fcollonval merged commit 56009e0 into jupyterlab:main Sep 26, 2023
75 of 79 checks passed
@fcollonval fcollonval deleted the bubbling_keydown branch September 26, 2023 08:39
@jtpio
Copy link
Member

jtpio commented Sep 26, 2023

Is this a new setting that end users should be able to enable, or is it mostly for helping with #14115? Should there be a mention somewhere in the JupyterLab documentation?

@krassowski
Copy link
Member

I think we don't know the answer yet. If we find it to work well I would not expose it to users. If we find any issues then it might be worth letting user decide. I would suggest opening an issue to come back to it before releasing 4.1.

@jtpio
Copy link
Member

jtpio commented Sep 26, 2023

Opened #15158

krassowski added a commit to krassowski/jupyterlab that referenced this pull request Sep 26, 2023
fcollonval added a commit that referenced this pull request Oct 3, 2023
* Add plugin manager and extension locking

* Add tests for pluginmanger, label, and mark promise with void

Add more tests for model

* Integrity fixes

* Remember the disclaimer checkbox state within session

and fix restoration of search query.

* Remove unneeded (and experimental) text-wrap

* Allow to search plugins by token name

* Add UI tests for plugin manager

* Move plugin manager tests to documentation

* Document plugin manager, move `jupyter labextension` section

* Update Playwright Snapshots

* Replace performance-limiting `:hover > td` rule with mix-blend

This was caught by a test on CI 🎉

* Update Playwright Snapshots

* Fix table header display

* Fix typos in documentation

Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>

* Apply suggestions from code review

Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove not needed test config

* Propagate frozen as needed, add imports

* Fix styling of table in dark mode

* Use frozenset

* Move table and icon to ui-components

* Use `Readonly<Pick>` for plugin data interface

* Support defer value, fix temporary icon shim

* Add tooltip with hints, lint previous commit

* Add quotes around plugin id in dialogs

* Use `React.PropsWithChildren`

* Clear error before API call, move up in class

* Move attributes up in the class definition

* Use `VDomRenderer` where possible

* Do not add plugin manager to menu, place it in settings toolbar

* Add migration entry for plugin manager and locks

* Integrity updates

* Update Playwright Snapshots

* Update icon identifier in tests

* Add plugin manager to optional tokens

* Revert menu settings snapshot update

* Lint

* Update documentation screenshot

* Update to latest `@lumino/widgets` version following #15142

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment