data-menu: enforce layer order (#3289)

* LayerSelect to support sort_by='zorder' or 'icon'
* sort data-menu by z-order

CHANGES.rst

@@ -4,7 +4,7 @@
 New Features
 ------------
 
-* New design for viewer legend and data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274]
+* New design for viewer legend and data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289]
 
 Cubeviz
 ^^^^^^^

jdaviz/configs/default/plugins/data_menu/data_menu.py

@@ -109,6 +109,7 @@ def __init__(self, viewer, *args, **kwargs):
         self.layer.remove_filter('filter_is_root')
         self.layer.add_filter(is_not_wcs_only)
         self.layer.multiselect = True
+        self.layer.sort_by = 'zorder'
         self.layer._default_mode = 'empty'
 
         # we'll use a modified version of the dataset mixin to have a filtered
@@ -227,8 +228,7 @@ def _dm_layer_selected_changed(self, event={}):
         with self.during_select_sync():
             # map index in dm_layer_selected (inverse order of layer_items)
             # to set self.layer.selected
-            length = len(self.layer_items)
-            self.layer.selected = [self.layer_items[length-1-i]['label']
+            self.layer.selected = [self.layer_items[i]['label']
                                    for i in self.dm_layer_selected]
 
     @observe('layer_selected', 'layer_items')
@@ -238,7 +238,7 @@ def _layers_changed(self, event={}):
         if not self._during_select_sync:
             with self.during_select_sync():
                 # map list of strings in self.layer.selected to indices in dm_layer_selected
-                layer_labels = [layer['label'] for layer in self.layer_items][::-1]
+                layer_labels = [layer['label'] for layer in self.layer_items]
                 self.dm_layer_selected = [layer_labels.index(label) for label in self.layer.selected
                                           if label in layer_labels]
 

jdaviz/configs/default/plugins/data_menu/data_menu.vue

@@ -33,7 +33,7 @@
                 <span class="invert-if-dark" style="margin-left: 30px; margin-right: 36px; line-height: 28px">{{viewer_reference || viewer_id}}</span>
               </div>
 
-              <div v-for="item in layer_items.slice().reverse()" class="viewer-label">
+              <div v-for="item in layer_items" class="viewer-label">
                 <div v-if="item.visible">
                   <span style="float: right">
                     <j-layer-viewer-icon-stylized
@@ -159,7 +159,7 @@
             >
               <div>
               <v-list-item 
-                v-for="item in layer_items.slice().reverse()" 
+                v-for="item in layer_items" 
                 class="layer-select" 
               > 
                 <v-list-item-icon>

jdaviz/configs/default/tests/test_data_menu.py

@@ -128,15 +128,15 @@ def test_data_menu_remove_subset(specviz_helper, spectrum1d):
                                     6100 * spectrum1d.spectral_axis.unit),
                      combination_mode='new')
 
-    assert dm.layer.choices == ['test', 'test2', 'Subset 1', 'Subset 2']
+    assert dm.layer.choices == ['Subset 2', 'Subset 1', 'test2', 'test']
     dm.layer.selected = ['Subset 1']
     dm.remove_from_viewer()
 
     # subset visibility is set to false, but still appears in menu (unlike removing data)
-    assert dm.layer.choices == ['test', 'test2', 'Subset 1', 'Subset 2']
-    assert dm._obj.layer_items[2]['label'] == 'Subset 1'
+    assert dm.layer.choices == ['Subset 2', 'Subset 1', 'test2', 'test']
+    assert dm._obj.layer_items[1]['label'] == 'Subset 1'
     # TODO: sometimes appearing as mixed right now, known bug
-    assert dm._obj.layer_items[2]['visible'] is not True
+    assert dm._obj.layer_items[1]['visible'] is not True
 
     # selection should not have changed by removing subset from viewer
     assert dm.layer.selected == ['Subset 1']
@@ -178,7 +178,7 @@ def test_data_menu_view_info(specviz_helper, spectrum1d):
                                     6300 * spectrum1d.spectral_axis.unit),
                      combination_mode='new')
 
-    assert dm.layer.choices == ['test', 'test2', 'Subset 1', 'Subset 2']
+    assert dm.layer.choices == ['Subset 2', 'Subset 1', 'test2', 'test']
 
     dm.layer.selected = ["test2"]
     dm.view_info()

jdaviz/core/template_mixin.py

@@ -1438,6 +1438,7 @@ class LayerSelect(SelectPluginComponent):
         hint="Select layer."
       />
     """
+    sort_by = Unicode('icon').tag(sync=True)
 
     def __init__(self, plugin, items, selected, viewer,
                  multiselect=None,
@@ -1446,7 +1447,8 @@ def __init__(self, plugin, items, selected, viewer,
                  only_wcs_layers=False,
                  is_root=True,
                  has_children=False,
-                 is_child_of=None):
+                 is_child_of=None,
+                 sort_by='icon'):
         """
         Parameters
         ----------
@@ -1469,6 +1471,10 @@ def __init__(self, plugin, items, selected, viewer,
         default_mode : str, optional
             What mode to use when making the default selection.  Valid options: first, default_text,
             empty.
+        sort_by : str, optional
+            How to sort the ordering of items.  Valid options: zorder (top layers are first),
+            icon (alphabetical by icon, effectively by order in which layers were first
+            added and assigned an icon)
         """
         super().__init__(plugin,
                          items=items,
@@ -1491,6 +1497,7 @@ def __init__(self, plugin, items, selected, viewer,
         self.hub.subscribe(self, SubsetDeleteMessage,
                            handler=lambda _: self._update_layer_items())
 
+        self.sort_by = sort_by
         self.app.state.add_callback('layer_icons', self._update_layer_items)
         self.add_observe(viewer, self._on_viewer_selected_changed)
         self.add_observe(selected, self._update_layer_items)
@@ -1569,6 +1576,7 @@ def not_trace(lyr):
     def _layer_to_dict(self, layer_label):
         is_subset = None
         subset_type = None
+        zorder = None
         from_plugin = None
         live_plugin_results = None
         colors = []
@@ -1582,6 +1590,8 @@ def _layer_to_dict(self, layer_label):
                                      (hasattr(layer, 'layer') and hasattr(layer.layer, 'subset_state')))  # noqa
                         if is_subset:
                             subset_type = get_subset_type(layer.layer)
+                    if zorder is None:
+                        zorder = layer.state.zorder
                     if from_plugin is None:
                         from_plugin = layer.layer.data.meta.get('Plugin', None)
                     if live_plugin_results is None:
@@ -1600,6 +1610,7 @@ def _layer_to_dict(self, layer_label):
         return {"label": layer_label,
                 "is_subset": is_subset,
                 "subset_type": subset_type,
+                "zorder": zorder,
                 "from_plugin": from_plugin,
                 "live_plugin_results": live_plugin_results,
                 "icon": self.app.state.layer_icons.get(layer_label),
@@ -1631,6 +1642,7 @@ def _on_viewer_selected_changed(self, msg=None):
                     if is_wcs_only(layer.layer):
                         continue
                     layer.remove_callback('color', self._update_layer_items)
+                    layer.remove_callback('zorder', self._update_layer_items)
                     if hasattr(layer, 'cmap'):
                         layer.remove_callback('cmap', self._update_layer_items)
                     if hasattr(layer, 'bitmap_visible'):
@@ -1649,6 +1661,7 @@ def _on_viewer_selected_changed(self, msg=None):
                     if is_wcs_only(layer.layer):
                         continue
                     layer.add_callback('color', self._update_layer_items)
+                    layer.add_callback('zorder', self._update_layer_items)
                     if hasattr(layer, 'cmap'):
                         layer.add_callback('cmap', self._update_layer_items)
                     if hasattr(layer, 'bitmap_visible'):
@@ -1693,7 +1706,7 @@ def _on_data_added(self, msg=None):
 
         self._update_layer_items({'source': 'data_added'})
 
-    @observe('filters')
+    @observe('filters', 'sort_by')
     def _update_layer_items(self, msg={}):
         # NOTE: _on_layers_changed is passed without a msg object during init
         # TODO: Handle changes to just one item without recompiling the whole thing
@@ -1721,7 +1734,14 @@ def _sort_by_icon(items_dict):
             icon = items_dict['icon']
             return icon if icon is not None else ''
 
-        layer_items.sort(key=_sort_by_icon)
+        def _sort_by_zorder(items_dict):
+            # NOTE: this works best if subscribed to a single viewer
+            return -1 * items_dict.get('zorder', 0)
+
+        if self.sort_by == 'zorder':
+            layer_items.sort(key=_sort_by_zorder)
+        else:  # icon
+            layer_items.sort(key=_sort_by_icon)
 
         self.items = manual_items + layer_items