Framework for legend data menu

[kecnry]

Description

This pull request implements the framework for the redesigned data-menu launched from the viewer legend - currently including a list of layers with togglable visibilities. Until this reaches feature-parity with the existing data menu and can be launched in its place, this will be hidden behind a developer flag to avoid confusion. To enable for testing (note: both this and the _obj will be removed when making the data-menu public):

for viewer in viz.viewers.values():
    viewer._obj.data_menu._obj.dev_data_menu = True

TODO / follow-ups:

• implement as much styling as applicable from the UI design
  • set color of icons in header/footer (will be in next PR)
  • use "toolbar" color for dark mode toggling instead of hardcoding
  • show subset icon where applicable
• fix index > selection mapping when sublayer is included in the list
• fix data-menu selection when adding new layers
• investigate subset visibility lag and determine if an upstream bug
• investigate assigning spectral subsets in cube viewer as temporal and create ticket
• sublayer support (some may be deferred)
  • nest sublayers in UI or keep by z-order? (talked to @Jenneh - let's defer for now and she'll work on a design)
  • toggling visibility of parent should disable visibility of children (see behavior in current data menu)

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

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 milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[kecnry]

this was never exposed in the user API, so probably doesn't need to be deprecated? Although we may want to mention in explicitly in the changelog (hiding viewer labels is now longer supported since they will soon become the trigger for opening the data menu).

[codecov]

Codecov Report

Attention: Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.66%. Comparing base (236f75a) to head (3fc5293).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...viz/configs/default/plugins/data_menu/data_menu.py	95.65%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3254      +/-   ##
==========================================
+ Coverage   88.63%   88.66%   +0.03%     
==========================================
  Files         125      125              
  Lines       18779    18814      +35     
==========================================
+ Hits        16644    16681      +37     
+ Misses       2135     2133       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[javerbukh]

Overall I think this is great! Much more intuitive design than what we have currently. I also really like the "Coming Soon" labels, it was very helpful during testing to know if something was supposed to be implemented or not. Some other observations:

1. There's no way to close the menu. Clicking away works but it would nice if there was a more explicit interaction.
2. It was not obvious how to get a spectral subset to show up in the menu. When it did show up, the only way to toggle the visibility was to click on the eye icon and then the letter "C" that represents the Spectrum.

Please ping me when you're ready for another look, very excited to see where this goes!

[kecnry]

There's no way to close the menu. Clicking away works but it would nice if there was a more explicit interaction.

What do you think of the latest change that swaps the viewer number (or adds a new entry) for a close button when the viewer is opened?

Screen.Recording.2024-10-29.at.1.04.25.PM.mov

It was not obvious how to get a spectral subset to show up in the menu. When it did show up, the only way to toggle the visibility was to click on the eye icon and then the letter "C" that represents the Spectrum.

Yep, spectral subsets are definitely buggy. We'll definitely need to fix this before making the new menu public, but I think its just exposing existing buggy functionality with callbacks and messages.

[gibsongreen]

Everything is coming along really well! In testing some of the other plugins outside of what we saw in demos, I noticed a couple of things that may be outside of the scope for this effort:

1. The ticket referenced indentation/grouping for child layers (and making follow-up tickets for it), I tested this with the DQ layer and wanted to make sure this is out of scope for now.

2. With gaussian smooth and spectral extraction, the data items do get added to the spectrum-viewer data menu. I also tested with a moment map and collapse, and it does getting added to the new viewer if I make select which image-viewer to plot in. But, if I don’t chose that option, it will load in the old data menu (not in viewer), and then if I add it to the viewer it then populates the new data menu.

[kecnry]

The ticket referenced indentation/grouping for child layers (and making follow-up tickets for it), I tested this with the DQ layer and wanted to make sure this is out of scope for now.

Yes, the child layers should show with the correct notation (A1, etc), but we are intentionally deferring any indentation until @Jenneh advises.

With gaussian smooth and spectral extraction, the data items do get added to the spectrum-viewer data menu. I also tested with a moment map and collapse, and it does getting added to the new viewer if I make select which image-viewer to plot in. But, if I don’t chose that option, it will load in the old data menu (not in viewer), and then if I add it to the viewer it then populates the new data menu.

Correct, this currently only shows data already loaded in the viewer. The "+" button on the top right will eventually handle loading additional available data into the viewer, similar to the "show data not in viewer" section of the old data menu. See the mockup for more details.

[bmorris3]

This looks great! Here are some comments for iterating over the Epic. They don't necessarily preclude approval of this PR.

1. Can we put the visibility toggle on the left? I think one standard UI pattern for comparison is photoshop's layers tool:

2. The subset icons seem to appear on some layer select updates:

data-menu-subset-icon.mov

3. The data layer (multi-)selection is great. I wish it were easier to tell if all layers are selected though. Here's with none selected:

and here's with all selected:

For example, could we add weight to the font on selected layers?

4. Something's going on with mixed visibility states in subsets. Is this expected?:

data-menu-subset-mixed-visibility_720p.mov

[kecnry]

Can we put the visibility toggle on the left?

@Jenneh - any thoughts? The list-action-item component that we use from vue does generally have action buttons on the right, but we can force it to be different if we want.

The subset icons seem to appear on some layer select updates

Yes, this is the known bug that we're hoping is improved by changes in upstream subset messaging.

I wish it were easier to tell if all layers are selected though. For example, could we add weight to the font on selected layers?

That still might have the same problem, but I like it to make them stand out more. I'll give it a shot and see how it looks. Note that in the follow-up to enable actions in the bottom bar, I think this will become more intuitive, since those will be context dependent on the selection.

Something's going on with mixed visibility states in subsets. Is this expected?

It is known, yes (but not introduced here... as you can see, similar problems exist in the old data menu and legend)

[kecnry]

@bmorris3 - how about this?

[bmorris3]

With the exception of the possible rebase issue noted below, the code looks great and ready to go.

[gibsongreen]

After a bit more testing and code review I didn't see anything new that hasn't been addressed in the discussion above!

[javerbukh]

All comments have been addressed and follow-up tickets have been made, so LGTM. Looking forward to seeing the work on this continue!