Nested viewer toolbars

[kecnry]

Description

This pull request implements a nested dropdown toolbar so that we can move the plot options directly into the toolbar and avoid covering the plot itself. In doing so, it also moves the "save plot" and "plot options" to new plugins in the sidebar, which can be opened to a given viewer with new tools in the toolbar (to the far right).

The new "nested toolbar" consists of up to 6 categories that should hopefully encompass any tools we develop in the future:

• zoom reset (currently just home)
• zoom tools
• pan tools
• subset selection
• other tools (slice select, line select, contrast/bias, blink)
• plugin tray shortcuts (plot options, save, compass)

In order to avoid overflow onto two lines in cubeviz when the sidebar is opened, this also replaces the "DATA" button with an icon (icon itself up for debate/redesign). For the image viewers in cubeviz, the "zoom" and "pan" tools have also been merged to save space.

Screen.Recording.2022-03-07.at.10.56.29.AM.mov

Imviz with shortcuts to per-viewer plot options, saving, and compass:

Screen.Recording.2022-03-07.at.10.57.52.AM.mov

The behavior of the persistent slice indicator has also been slightly "improved". When clicking on a "non-checkable" tool (home, plugin shortcuts, blink), the previously selected tool, if any, will be re-activated. If unchecking a "checkable" tool, then the toolbar will choose the first from a list of defaults that is the primary tool in its category. For cubeviz, this will currently always default to the slice indicator, but in the future may choose between the slice indicator and spectral line selector (see #1115), depending on which is currently active.

Non-checkable tools (plugin-shortcuts) just act immediately and do not replace the primary item in the toolbar. We may need to reconsider this in the future and replace the plugin shortcuts with dropdown menu outside the toolbar.

Blink is now a checkable tool to feel consistent. You enable the tool and then click anywhere in the viewer to blink.

TODO:

• fix repr of imviz viewer
• reconnect persistent slice tool in cubeviz
• whiter icons (override views dimming since we use orange background for active)
• when clicking on a non-checkable tool, revert to the previous selection (currently will revert to any default tools)
• decide if "primary" entry should re-appear in submenu
• if "primary" entry in submenu, show as active to indicate its already selected? How to differentiate between "primary" and "primary AND active"?
• when clicking on a non-checkable tool from the sub-menus, should the action immediately trigger or require a second click (only set it as the primary and then require clicking to activate)? Should the plugin shortcuts be a normal dropdown menu with NO primary tool (so a kabab menu)?
• reconsider icon for save/export plot
• test coverage
• docs updates, including links from new plugins to entries in the docs

FUTURE WORK

• show reference names instead of ids, when available, in viewer dropdowns. Might be useful to have a reusable component for this now that we have several plugins with the same pattern.
• clicking on plugin shortcut should scroll to that item (attempted - not simple)
• long-press (in addition to the existing right-click) support (attempted - not simple)
• blink tool to become "checkable" and act on click on the plot rather than click on the tool. This would make its behavior more consistent with other tools in the same group (see also idea to have a unified toolbar below which would require this)
• new reset-ylims tool (in same group as home)
• zoom back button (in same group as home - won't be easy because drag events might make storing history difficult)
• replace viewer.tools/viewer.toolbar and perhaps move implementation upstream, if interested
• consider merging to a unified app-level toolbar. This will take some consideration where some tools are only applicable to certain viewers and will need to re-factor the activation logic to activate in all relevant viewers simultaneously. The toolbar could be built based on the tools requested from each viewer, activated for any applicable viewers, and any non-applicable viewers would show a disabled mouse icon. All non-checkable tools (home, plugin shortcuts, blink, etc) would need to become checkable, with the mouse turning into that icon and then could click on any viewer to apply to that viewer. If we went in this direction, we'd probably want to re-consider the persistent slice indicator and possibly use glue's viewer._default_mouse_mode_cls instead.
• developer docs for adding new tools, categories, etc.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a change log needed? If yes, is it added to CHANGES.rst?
• Is a milestone set? Milestone is only currently required for PRs related to Imviz MVP.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[kecnry]

CI is failing because of the older pinned version of glue-jupyter. Hopefully updating that pin when 0.11.1 is out should clear up most, if not all, issues.

[pllim]

Are you using a new feature only available in 0.11?

[kecnry]

Yes, I need the new support for assigning templates that was added in 0.11.

[codecov]

Codecov Report

Merging #1140 (8ed7c62) into main (a35f93a) will increase coverage by 0.06%.
The diff coverage is 79.89%.

@@            Coverage Diff             @@
##             main    #1140      +/-   ##
==========================================
+ Coverage   76.66%   76.72%   +0.06%     
==========================================
  Files          82       87       +5     
  Lines        6483     6655     +172     
==========================================
+ Hits         4970     5106     +136     
- Misses       1513     1549      +36     

Impacted Files	Coverage Δ
jdaviz/app.py	89.98% <ø> (+1.02%)	⬆️
jdaviz/configs/default/plugins/viewers.py	82.14% <61.53%> (-17.86%)	⬇️
jdaviz/configs/imviz/plugins/tools.py	44.66% <66.66%> (-0.34%)	⬇️
jdaviz/components/toolbar_nested.py	72.22% <72.22%> (ø)
jdaviz/core/tools.py	76.04% <78.37%> (+1.46%)	⬆️
...configs/default/plugins/export_plot/export_plot.py	80.00% <80.00%> (ø)
jdaviz/configs/mosviz/plugins/viewers.py	82.38% <80.00%> (-0.17%)	⬇️
...nfigs/default/plugins/plot_options/plot_options.py	96.55% <96.55%> (ø)
jdaviz/configs/cubeviz/plugins/tools.py	73.33% <100.00%> (-10.00%)	⬇️
jdaviz/configs/cubeviz/plugins/viewers.py	97.61% <100.00%> (+2.38%)	⬆️
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a35f93a...8ed7c62. Read the comment docs.

[javerbukh]

Quick note, right clicking on the trackpad of a macbook in jupyter-lab opens the lab right click options as well.

[javerbukh]

Right click options also appear very faint in dark mode

[kecnry]

@javerbukh - dark mode strikes again! Thanks for catching both of those - they should be fixed now.

[javerbukh]

@kecnry That works thanks! I'll continue with the review. One other thing, the orange box seems to be off a little bit from where I would expect it to be:

[kecnry]

Does new jdaviz.components module needs to be listed under API doc page?

That's a good question. I try to keep everything there to be strictly UI logic, so I don't think any user would ever need to access API for those (and until now, it has been strictly .vue files, no python).

Will this affect MAST viz?

I don't see any reason why something here that works in jupyter won't work when embedded, but I was also surprised by cross-browser issues, so extra testing never hurts!

[kecnry]

Any chance that the button to open the plot options could also close the plugin tray if clicked a second time?

It's possible to implement, but with a few complications: right now the plugin shortcuts aren't "checkable" (they don't become active/orange). Clicking on another shortcut from another viewer still updates the viewer dropdown, which I think is useful, and we definitely don't want that to close the sidebar. The two options would be to make them "checkable" (I don't think we want that) or to check to see if that plugin is open and with that viewer selected, and if so, close the sidebar. Honestly, I'm hoping we eventually get rid of these buttons in favor of a more robust "viewer selector" similar to #1115 and see them as just helping users with the change in the meantime.

[pllim]

I don't know how to blink now using the new GUI. Left click turns it on and off. It used to blink when I click the button. Luckily the "b" key binding still works. (Doh, I was clicking on the button, not the image.)

Contrast/bias adjustment seems to be broken too. Instead of adjusting, it draws a region. Works with new bqplot-image-gl, see #1149

Might be a problem when people open too many viewers in Imviz (Kyle said this will fix itself when we change the toolbar design again):

Now we seem to have two different ways to open the same plugin menu. Do we really need both? (Kyle said this will fix itself when we change the toolbar design again.)

[pllim]

At least for Imviz, I propose rearranging the order to reflect better typical workflow logic. You want to adjust display first but save the plot last.

Suggested change

	- g-metadata-viewer
	- g-export-plot
	- g-plot-options
	- g-plot-options
	- g-metadata-viewer

[pllim]

As per offline convo, we can defer this.

[pllim]

Suggested change

	- imviz-aper-phot-simple
	- imviz-aper-phot-simple
	- g-export-plot

[duytnguyendtn]

Wanted to add my quick two cents, but since we already have Jesse and Pey Lian's review, I won't go too deep into a full approval and rely on their more thorough reviews. On the surface, everything looks good on my side; everything is interactable the way I expected it to, and I don't see the scrollbar issue that Ricky's seeing

Python 3.9.7 (tags/v3.9.7:1016ef3, Aug 30 2021, 20:19:38) [MSC v.1929 64 bit (AMD64)] on win32, using Chromium: 98.0.4758.102