plugin active status based on pings (#2295)

* plugin active status determined by "alive" pings

* code cleanup

* fix broken tests

* don't crash if line analysis fails in specutils

* mosviz.test_parsers was failing with an interpolation error on initial load

* use plugin_opened = True in tests instead of open_in_tray()

* consider "hidden" plugin to be inactive

* other tab, minimized window, window completely behind window, etc. 
* prevents "flickering" caused by browser throttling of hidden windows

* replace ping plugin with method

* to avoid unnecessary traitlet syncing

* separate changelog entry for API deprecation

* allow skipping keep_active UI

by not passing to template, this then means all plugins should listen to is_active and never plugin_opened

* update compass to use is_active

* update line_profile to use is_active

* update plot options to use is_active

* more verbose test for line assignment

CHANGES.rst

@@ -67,6 +67,9 @@ API Changes
 - ``viz.get_data()`` now takes optional ``**kwargs``; e.g., you could pass in
   ``function="sum"`` to collapse a cube in Cubeviz. [#2242]
 
+- Live-previews and keypress events that depend on the plugin being opened now work for inline
+  and popout windows. [#2295]
+
 Cubeviz
 ^^^^^^^
 
@@ -99,6 +102,9 @@ Bug Fixes
 
 - Fixed ``cls`` input being ignored in ``viz.get_data()``. [#2242]
 
+- Line analysis plugin's ``show_continuum_marks`` is deprecated, use ``plugin.as_active()`` 
+  instead. [#2295]
+
 Cubeviz
 ^^^^^^^
 

docs/dev/ui_style_guide.rst

@@ -8,9 +8,11 @@ Tray Plugins
 In order to be consistent with layout, styling, and spacing, UI development on plugins should
 try to adhere to the following principles:
 
-* Any tray plugin should utilize ``<j-tray-plugin :disabled_msg='disabled_msg'>`` as the
-  outer-container (which provides consistent styling rules).  Any changes to style
-  across all plugins should then take place in the
+* Any tray plugin should utilize ``<j-tray-plugin :disabled_msg='disabled_msg' :popout_button="popout_button">`` as the
+  outer-container (which provides consistent styling rules).  If the plugin makes use of active status
+  (live-preview marks or viewer callbacks), then also pass `` :uses_active_status="uses_active_status" @plugin-ping="plugin_ping($event)"``.
+  To enable the "keep active" check, pass `` :uses_active_status="uses_active_status" @plugin-ping="plugin_ping($event)" :keep_active.sync="keep_active" ``.
+  Any changes to style across all plugins should then take place in the
   ``j-tray-plugin`` stylesheet (``jdaviz/components/tray_plugin.vue``).
 * Each item should be wrapped in a ``v-row``, but avoid any unnecessary additional wrapping-components
   (``v-card-*``, ``v-container``, etc).

jdaviz/app.vue

@@ -120,7 +120,7 @@
                       </j-tooltip>
                     </v-expansion-panel-header>
                     <v-expansion-panel-content style="margin-left: -12px; margin-right: -12px;">
-                      <jupyter-widget :widget="trayItem.widget"></jupyter-widget>
+                      <jupyter-widget :widget="trayItem.widget" v-if="state.tray_items_open.includes(index)"></jupyter-widget>
                     </v-expansion-panel-content>
                   </div>
                 </v-expansion-panel>

jdaviz/components/tray_plugin.vue

@@ -13,22 +13,54 @@
       <span> {{ getDisabledMsg() }}</span>
     </v-row>
     <div v-else>
+      <v-row v-if="uses_active_status && keep_active !== undefined">
+        <v-switch
+          v-model="keep_active"
+          @change="$emit('update:keep_active', $event)"
+          label="keep active"
+          hint="consider plugin active (showing any previews and enabling all keypress events) even when not opened"
+          persistent-hint>
+        </v-switch>
+      </v-row>
       <slot></slot>
     </div>
   </v-container>
 </template>
 
 <script>
 module.exports = {
-  props: ['disabled_msg', 'description', 'link', 'popout_button'],
+  props: ['disabled_msg', 'description', 'link', 'popout_button',
+          'uses_active_status', 'keep_active'],
   methods: {
     isDisabled() {
       return this.getDisabledMsg().length > 0
     },
     getDisabledMsg() {
       return this.disabled_msg || ''
     },
-  }
+    sendPing(recursive) {
+      if (!this.$el.isConnected) {
+        return
+      }
+      if (!document.hidden) {
+        this.$emit('plugin-ping', Date.now())
+      }
+      if (!recursive) {
+        return
+      }
+      setTimeout(() => {
+        this.sendPing(true)          
+      }, 200)  // ms
+    }
+  },
+  mounted() {
+    this.sendPing(true);
+    document.addEventListener("visibilitychange", () => {
+      if (!document.hidden) {
+        this.sendPing(false)
+      }
+    });
+  },
 };
 </script>
 

jdaviz/configs/cubeviz/plugins/tests/test_tools.py

@@ -15,13 +15,13 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube):
     flux_viewer.toolbar.active_tool = flux_viewer.toolbar.tools['jdaviz:spectrumperspaxel']
     x = 1
     y = 1
-    assert len(flux_viewer.figure.marks) == 2
+    assert len(flux_viewer.native_marks) == 2
     assert len(spectrum_viewer.data()) == 1
 
     # Click on spaxel location
     flux_viewer.toolbar.active_tool.on_mouse_event(
         {'event': 'click', 'domain': {'x': x, 'y': y}, 'altKey': False})
-    assert len(flux_viewer.figure.marks) == 3
+    assert len(flux_viewer.native_marks) == 3
     assert len(spectrum_viewer.data()) == 2
 
     # Check that a new subset was created
@@ -32,7 +32,7 @@ def test_spectrum_at_spaxel(cubeviz_helper, spectrum1d_cube):
 
     # Deselect tool
     flux_viewer.toolbar.active_tool = None
-    assert len(flux_viewer.figure.marks) == 3
+    assert len(flux_viewer.native_marks) == 3
 
 
 def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
@@ -46,7 +46,7 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
     flux_viewer.toolbar.active_tool = flux_viewer.toolbar.tools['jdaviz:spectrumperspaxel']
     x = 1
     y = 1
-    assert len(flux_viewer.figure.marks) == 2
+    assert len(flux_viewer.native_marks) == 2
     assert len(spectrum_viewer.data()) == 1
 
     # Check coordinate info panel
@@ -60,7 +60,7 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
     # Click on spaxel location
     flux_viewer.toolbar.active_tool.on_mouse_event(
         {'event': 'click', 'domain': {'x': x, 'y': y}, 'altKey': False})
-    assert len(flux_viewer.figure.marks) == 3
+    assert len(flux_viewer.native_marks) == 3
     assert len(spectrum_viewer.data()) == 2
 
     # Check that subset was created
@@ -74,7 +74,7 @@ def test_spectrum_at_spaxel_altkey_true(cubeviz_helper, spectrum1d_cube):
     y = 2
     flux_viewer.toolbar.active_tool.on_mouse_event(
         {'event': 'click', 'domain': {'x': x, 'y': y}, 'altKey': True})
-    assert len(flux_viewer.figure.marks) == 4
+    assert len(flux_viewer.native_marks) == 4
     assert len(spectrum_viewer.data()) == 3
 
     subsets = cubeviz_helper.app.get_subsets()
@@ -118,15 +118,15 @@ def test_spectrum_at_spaxel_with_2d(cubeviz_helper):
     flux_viewer.toolbar.active_tool = flux_viewer.toolbar.tools['jdaviz:spectrumperspaxel']
     x = 1
     y = 1
-    assert len(flux_viewer.figure.marks) == 2
+    assert len(flux_viewer.native_marks) == 2
     assert len(spectrum_viewer.data()) == 0
 
     # Click on spaxel location
     flux_viewer.toolbar.active_tool.on_mouse_event(
         {'event': 'click', 'domain': {'x': x, 'y': y}, 'altKey': False})
-    assert len(flux_viewer.figure.marks) == 3
+    assert len(flux_viewer.native_marks) == 3
     assert len(spectrum_viewer.data()) == 0
 
     # Deselect tool
     flux_viewer.toolbar.active_tool = None
-    assert len(flux_viewer.figure.marks) == 3
+    assert len(flux_viewer.native_marks) == 3

jdaviz/configs/default/plugins/line_lists/line_lists.py

@@ -322,12 +322,7 @@ def _on_rs_redshift_updated(self, event):
             msg = RedshiftMessage("redshift", value, sender=self)
             self.app.hub.broadcast(msg)
 
-    @observe('plugin_opened')
     def _update_line_list_obs(self, *args):
-        if not self.plugin_opened:
-            return
-
-        new_list_contents = {}
         for list_name, line_list in self.list_contents.items():
             for i, line in enumerate(line_list['lines']):
                 if self._rs_line_obs_change[0] == list_name and self._rs_line_obs_change[1] == i:  # noqa
@@ -339,10 +334,9 @@ def _update_line_list_obs(self, *args):
                 else:
                     line_list['lines'][i]['obs'] = self._rest_to_obs(float(line['rest']))
 
-            new_list_contents[list_name] = line_list
+            self.list_contents[list_name] = line_list
 
-        self.list_contents = {}
-        self.list_contents = new_list_contents
+        self.send_state('list_contents')
 
     def vue_change_line_obs(self, kwargs):
         # NOTE: we can only pass one argument from vue (it seems), so we'll pass as

jdaviz/configs/default/plugins/line_lists/tests/test_line_lists.py

@@ -47,13 +47,13 @@ def test_line_lists(specviz_helper):
 
 def test_redshift(specviz_helper, spectrum1d):
     # Also test that plugin is disabled before data is loaded.
-    plg = specviz_helper.plugins['Line Lists']
-    assert plg._obj.disabled_msg
+    ll_plugin = specviz_helper.plugins['Line Lists']._obj
+    assert ll_plugin.disabled_msg
 
     label = "Test 1D Spectrum"
     specviz_helper.load_data(spectrum1d, data_label=label)
 
-    assert not plg._obj.disabled_msg
+    assert not ll_plugin.disabled_msg
 
     lt = QTable()
     lt['linename'] = ['O III', 'Halpha']
@@ -63,8 +63,7 @@ def test_redshift(specviz_helper, spectrum1d):
     with pytest.warns(UserWarning, match='per line/list redshifts not supported, use viz.set_redshift'):  # noqa
         specviz_helper.load_line_list(lt)
 
-    ll_plugin = specviz_helper.app.get_tray_item_from_name('g-line-list')
-    # fake the plugin to be opened so that all updates run
+    # open the plugin so that all updates run
     ll_plugin.plugin_opened = True
     line = ll_plugin.list_contents['Test List']['lines'][0]
     assert_allclose(line['obs'], line['rest'])

jdaviz/configs/default/plugins/markers/markers.py

@@ -1,5 +1,5 @@
 import numpy as np
-from traitlets import observe
+from traitlets import Bool, observe
 
 from jdaviz.core.events import ViewerAddedMessage
 from jdaviz.core.marks import MarkersMark
@@ -24,6 +24,7 @@ class Markers(PluginTemplateMixin, ViewerSelectMixin, TableMixin):
     * :meth:`~jdaviz.core.template_mixin.TableMixin.export_table`
     """
     template_file = __file__, "markers.vue"
+    uses_active_status = Bool(True).tag(sync=True)
 
     _default_table_values = {'spectral_axis': np.nan,
                              'spectral_axis:unit': '',
@@ -79,7 +80,7 @@ def __init__(self, *args, **kwargs):
         self.hub.subscribe(self, ViewerAddedMessage, handler=self._on_viewer_added)
 
     def _create_viewer_callbacks(self, viewer):
-        if not self.plugin_opened:
+        if not self.is_active:
             return
 
         callback = self._viewer_callback(viewer, self._on_viewer_key_event)
@@ -106,14 +107,14 @@ def marks(self):
     def coords_info(self):
         return self.app.session.application._tools['g-coords-info']
 
-    @observe('plugin_opened')
-    def _on_plugin_opened_changed(self, *args):
+    @observe('is_active')
+    def _on_is_active_changed(self, *args):
         if self.disabled_msg:
             return
 
         # toggle visibility of markers
         for mark in self.marks.values():
-            mark.visible = self.plugin_opened
+            mark.visible = self.is_active
 
         # subscribe/unsubscribe to keypress events across all viewers
         for viewer in self.app._viewer_store.values():
@@ -122,7 +123,7 @@ def _on_plugin_opened_changed(self, *args):
                 continue
             callback = self._viewer_callback(viewer, self._on_viewer_key_event)
 
-            if self.plugin_opened:
+            if self.is_active:
                 viewer.add_event_callback(callback, events=['keydown'])
             else:
                 viewer.remove_event_callback(callback)

jdaviz/configs/default/plugins/markers/markers.vue

@@ -2,6 +2,9 @@
   <j-tray-plugin
     description='Create and export markers.  Press "m" with the cursor over a viewer to log the mouseover information.  To change the selected layer, click the layer cycler in the mouseover information section of the app-level toolbar.'
     :link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#markers'"
+    :uses_active_status="uses_active_status"
+    @plugin-ping="plugin_ping($event)"
+    :keep_active.sync="keep_active"
     :popout_button="popout_button">
 
     <jupyter-widget :widget="table_widget"></jupyter-widget> 

jdaviz/configs/default/plugins/markers/tests/test_markers_plugin.py

@@ -28,7 +28,7 @@ def test_markers_cubeviz(cubeviz_helper, spectrum1d_cube):
     label_mouseover = cubeviz_helper.app.session.application._tools['g-coords-info']
 
     mp = cubeviz_helper.plugins['Markers']
-    mp.open_in_tray()
+    mp.keep_active = True
 
     # test event in flux viewer
     label_mouseover._viewer_mouse_event(fv,
@@ -112,13 +112,13 @@ def test_markers_cubeviz(cubeviz_helper, spectrum1d_cube):
     assert len(_get_markers_from_viewer(fv).x) == 1
     assert len(_get_markers_from_viewer(sv).x) == 2
 
-    # markers hide when plugin closed
-    cubeviz_helper.app.state.drawer = False
+    # markers hide when plugin closed and keep_active = False
+    mp.keep_active = False
     assert _get_markers_from_viewer(fv).visible is False
     assert _get_markers_from_viewer(sv).visible is False
 
     # markers re-appear when plugin re-opened
-    mp.open_in_tray()
+    mp._obj.plugin_opened = True
     assert _get_markers_from_viewer(fv).visible is True
     assert _get_markers_from_viewer(sv).visible is True
     assert len(_get_markers_from_viewer(fv).x) == 1
@@ -136,7 +136,7 @@ def test_markers_layer_cycle(self):
         label_mouseover = self.imviz.app.session.application._tools['g-coords-info']
 
         mp = self.imviz.plugins['Markers']
-        mp.open_in_tray()
+        mp.plugin_opened = True
 
         # cycle through dataset options (used for both coords info and markers)
         assert label_mouseover.dataset.choices == ['auto', 'none',
@@ -217,7 +217,7 @@ def test_markers_custom_viewer(self):
         label_mouseover = self.imviz.app.session.application._tools['g-coords-info']
 
         mp = self.imviz.plugins['Markers']
-        mp.open_in_tray()
+        mp.plugin_opened = True
 
         nv = self.imviz.create_image_viewer()
         self.imviz.app.add_data_to_viewer('imviz-1', 'has_wcs[SCI,1]')

jdaviz/configs/default/plugins/plot_options/plot_options.py

@@ -100,6 +100,7 @@ class PlotOptions(PluginTemplateMixin):
     * :meth: `set_histogram_nbins`
     """
     template_file = __file__, "plot_options.vue"
+    uses_active_status = Bool(True).tag(sync=True)
 
     # multiselect is shared between viewer and layer
     multiselect = Bool(False).tag(sync=True)
@@ -484,7 +485,7 @@ def vue_set_value(self, data):
         value = data.get('value')
         setattr(self, attr_name, value)
 
-    @observe('plugin_opened', 'layer_selected', 'viewer_selected',
+    @observe('is_active', 'layer_selected', 'viewer_selected',
              'stretch_hist_zoom_limits')
     def _update_stretch_histogram(self, msg={}):
         # Import here to prevent circular import.
@@ -497,7 +498,7 @@ def _update_stretch_histogram(self, msg={}):
         if not hasattr(self, 'viewer'):  # pragma: no cover
             # plugin hasn't been fully initialized yet
             return
-        if (not self.plugin_opened
+        if (not self.is_active
                 or not self.viewer.selected
                 or not self.layer.selected):  # pragma: no cover
             # no need to make updates, updates will be redrawn when plugin is opened

jdaviz/configs/default/plugins/plot_options/plot_options.vue

@@ -2,6 +2,8 @@
   <j-tray-plugin
     description='Viewer and data/layer options.'
     :link="'https://jdaviz.readthedocs.io/en/'+vdocs+'/'+config+'/plugins.html#plot-options'"
+    :uses_active_status="uses_active_status"
+    @plugin-ping="plugin_ping($event)"
     :popout_button="popout_button">
 
     <v-row>

jdaviz/configs/default/plugins/plot_options/tests/test_plot_options.py

@@ -65,7 +65,7 @@ def test_multiselect(cubeviz_helper, spectrum1d_cube):
 def test_stretch_histogram(cubeviz_helper, spectrum1d_cube_with_uncerts):
     cubeviz_helper.load_data(spectrum1d_cube_with_uncerts)
     po = cubeviz_helper.app.get_tray_item_from_name('g-plot-options')
-    po.open_in_tray()  # forces histogram to draw
+    po.plugin_opened = True
 
     assert po.stretch_histogram is not None
 

jdaviz/configs/default/plugins/viewers.py

@@ -21,7 +21,7 @@ class JdavizViewerMixin:
     toolbar = None
     tools_nested = []
     _prev_limits = None
-    _native_mark_classnames = ('Lines', 'LinesGL')
+    _native_mark_classnames = ('Lines', 'LinesGL', 'FRBImage', 'Contour')
 
     def __init__(self, *args, **kwargs):
         # NOTE: anything here most likely won't be called by viewers because of inheritance order