data-menu: show add data list for empty viewers

[kecnry]

Description

This pull request shows a list of data to add into a viewer when the viewer is empty in place of the previous data-menu's behavior of opening for new viewers.

This definitely could benefit from some more styling, but is needed to reach feature parity with the former data-menu so that it can be deprecated.

Screen.Recording.2024-11-07.at.3.56.32.PM.mov

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)?

[codecov]

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.76%. Comparing base (1f57249) to head (4ba5272).
Report is 8 commits behind head on main.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3272      +/-   ##
==========================================
- Coverage   88.76%   88.76%   -0.01%     
==========================================
  Files         125      125              
  Lines       18956    18963       +7     
==========================================
+ Hits        16826    16832       +6     
- Misses       2130     2131       +1     

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

[gibsongreen]

The UI looks good with the new updates, especially having an empty viewer that now enforces the user to load data and removing the ability to remove data entries across the entire app when in an empty viewer.

When a new viewer is created, I think there should still be a option for the multi-select option, but in a way that lets the user chose multiple datasets to load into the viewer at once.
.

Should we include a message similar to what is in the old data menu?

To show users where the new data menu is can we reintroduce auto-opening the data menu at app startup with the new menu?

This is on main where when you load a dataset that has DQ, when you toggle off the parent layer, the child layer remains visible.

Screen.Recording.2024-11-13.at.12.35.20.AM.mov

[kecnry]

When a new viewer is created, I think there should still be a option for the multi-select option, but in a way that lets the user chose multiple datasets to load into the viewer at once.

There currently is no plan for single select on visibility (this was for cubeviz, where cube viewers would default to changing which layer is visible instead of toggling visibilities manually).

Should we include a message similar to what is in the old data menu?

Good idea - I'll add that in this PR

To show users where the new data menu is can we reintroduce auto-opening the data menu at app startup with the new menu?

Possibly, but it is often then triggered when we programmatically open a viewer and then add data. Maybe instead we can highlight or add some animation to the viewer number which would open the menu? I think this can probably be deferred to the final UI/UX review with @Jenneh

This is on main where when you load a dataset that has DQ, when you toggle off the parent layer, the child layer remains visible.

I think that's out of scope for this PR, but I'll make a follow-up ticket. The design for child-layers is still up-for-debate, but I agree that the visibility logic should be maintained from the previous behavior for now.

[kecnry]

@gibsongreen - what do you think of this to help users find the right menu when there is no data?

Screen.Recording.2024-11-13.at.11.36.58.AM.mov

[gibsongreen]

@gibsongreen - what do you think of this to help users find the right menu when there is no data?

I like that a lot, it isn't overbearing on the user and definitely gets their attention to interact with the new menu!

Another idea might be to have this animation be present at app-startup even when data is already loaded until the user clicks the data menu for the first time in the app session (sort of coercing them to interact with it). Then deprecate this use of the animation at the next minor/major release once users had some time working with it.

[javerbukh]

This is looking good! I especially like the pulse you added for the button to add data. I think something like that would be beneficial for new users app-wide. The only issue I came across was when re-adding a viewer that had already been deleted. I can confirm that that bug does not exist on main.

1. Run imviz example notebook
2. Add new viewer (viewer 1)
3. Close viewer 1
4. Re-add new viewer
5. Try to select data

I then see the following traceback:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
File [~\anaconda3\envs\test-strauss\Lib\site-packages\ipyvue\VueTemplateWidget.py:60](http://localhost:8888/lab/tree/notebooks/~/anaconda3/envs/test-strauss/Lib/site-packages/ipyvue/VueTemplateWidget.py#line=59), in Events._handle_event(self, _, content, buffers)
     58     getattr(self, "vue_" + event)(data, buffers)
     59 else:
---> 60     getattr(self, "vue_" + event)(data)

File [~\Documents\STScI\jdaviz\jdaviz\configs\default\plugins\data_menu\data_menu.py:354](http://localhost:8888/lab/tree/notebooks/~/Documents/STScI/jdaviz/jdaviz/configs/default/plugins/data_menu/data_menu.py#line=353), in DataMenu.vue_add_data_to_viewer(self, info, *args)
    353 def vue_add_data_to_viewer(self, info, *args):
--> 354     self.add_data(info.get('data_label'))

File [~\Documents\STScI\jdaviz\jdaviz\configs\default\plugins\data_menu\data_menu.py:351](http://localhost:8888/lab/tree/notebooks/~/Documents/STScI/jdaviz/jdaviz/configs/default/plugins/data_menu/data_menu.py#line=350), in DataMenu.add_data(self, *data_labels)
    349     raise ValueError(f"Data labels {unavailable} not able to be loaded into '{self.viewer_id}'.  Must be one of: {self.dataset.choices}")  # noqa
    350 for data_label in data_labels:
--> 351     self.app.add_data_to_viewer(self.viewer_id, data_label)

File [~\Documents\STScI\jdaviz\jdaviz\app.py:1646](http://localhost:8888/lab/tree/notebooks/~/Documents/STScI/jdaviz/jdaviz/app.py#line=1645), in Application.add_data_to_viewer(self, viewer_reference, data_label, visible, clear_other_data)
   1644 viewer_item = self._get_viewer_item(viewer_reference)
   1645 if viewer_item is None:
-> 1646     raise ValueError(f"Could not identify viewer with reference {viewer_reference}")
   1648 self.set_data_visibility(viewer_item, data_label, visible=visible, replace=clear_other_data)

ValueError: Could not identify viewer with reference

[kecnry]

I like both those ideas to use the pulsating more - let's keep that in mind for future work!

@javerbukh - looking into that issue now - I suspect it wasn't added by this PR since this is mostly UI logic, so I may make it as a separate PR

[javerbukh]

Looks good!

[gibsongreen]

LGTM!