search

jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
596 stars 127 forks source link

Pass `_luminoEvent` argument when executing commands via keybinding #644

Closed andrewfulton9 closed 7 months ago

andrewfulton9 commented 8 months ago

Necessary for jupyterlab issue #15046 .

relevant discussion captured in #570.

Simply appends an _luminoEvent argument to args in _executeKeyBinding.

welcome[bot] commented 8 months ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

krassowski commented 8 months ago

What would be a good place(s) to document this new argument?

andrewfulton9 commented 8 months ago

That's a good question. It looks like the Lumino docs aren't much more than the API reference, but the function that is directly changed here is private and isn't in the docs. I could add something to the docstring for IKeyBindingOptions.args and maybe ICommandOptions.isEnabled and ICommandOptions.execute referencing the added argument in the case that the command is called from a keybinding. How does that sound?

andrewfulton9 commented 7 months ago

Thanks @andrewfulton9 it looks really nice. I like the idea of prefixing by _ added arguments.

Would you mind extending the following test to check it receives the new attribute with the correct value:

https://github.com/jupyterlab/lumino/blob/42b30f7f1b4b5ee1d1cec75583adc659b57f02bf/packages/commands/tests/src/index.spec.ts#L634

Thanks! I just added the test.

welcome[bot] commented 7 months ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart: