search

jupyterlab / lumino

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

Add overlay UI element #633

Closed g547315 closed 4 months ago

g547315 commented 9 months ago

References

#15090

Code changes

Added and styled Add overlay UI element

User-facing changes

Backwards-incompatible changes

None

krassowski commented 9 months ago

Before putting more work into this, it might benefit from some more discussion on how we want to implement this. I can imagine that we might in future want to add the shortcut overlays to other UI elements too. Maybe a more generic approach would be better? I can imagine using a data attribute on elements with shortcuts to be highlighted and then creating the overlays as needed. In particular this could also work for toolbar buttons without duplicating the code. As for the trigger (when the overlay is shown), possibly it could be a part of lumino keybindings processing loop?

CC @brichet who is working on rewriting toolbar buttons and also looked into keybindings recently to ask about thoughts on the implementation proposed above.

brichet commented 9 months ago

Thanks @krassowski for the ping. I agree that we should use a more generic approach. Using a data attribute on elements could be a good way of displaying the overlay on any element. This would work for toolbar buttons and should also work for extension elements.

AFAIK there is currently no link between an 'active' element (clickable) and its shortcut equivalent. For toolbar buttons the shortcuts could probably be inferred from the command (which are identical). However I don't know how it could be generalized to any element, like the tabbar, which is not described the same way...

g547315 commented 9 months ago

Do we have a proposed generic approach for the overlays that is suitable or can we use this current implementation as it provides value to the end user while this is looked further into.

Happy to refactor the code when that time comes, the only thing i would like to be considered is that the proposed solution has a css visibility attribute

@krassowski @brichet

tonyfast commented 9 months ago

i am trying to test this pull request on https://lumino--633.org.readthedocs.build/en/633/examples/dockpanel/index.html . i am not seeing anything happen when i press the prescribed key combinations. is there anyway to update to the examples with this working feature so its easier to test?

g547315 commented 8 months ago

i am trying to test this pull request on https://lumino--633.org.readthedocs.build/en/633/examples/dockpanel/index.html . i am not seeing anything happen when i press the prescribed key combinations. is there anyway to update to the examples with this working feature so its easier to test?

This by itself will just add a hidden div element called lm-TabBar-UI-Overlay to every tab on the side panels

@tonyfast

tonyfast commented 8 months ago 
* To see the interned functionality (without the short-cut number in the overlay) please combine this PR with:
  extend keystrokes to mod keys:[extend keystrokes to mod keys #637](https://github.com/jupyterlab/lumino/pull/637)

i do not have an easy way to test accessibility builds on multiple branches. it is really time consuming and technically challenging to build these results then test them with assistve tech. i'd really like to help move these pull requests forward, but their structure makes it really hard to test. is there anyway to combine these pull requests into something that can be tested easier? ideally testing the outcomes on the lumino examples would be best.

g547315 commented 5 months ago

This PR is replaced by this PR as it implements a more generic approach to overlays that is reusable