Skip to content

Commit

Permalink
Merge pull request #2394 from meeseeksmachine/auto-backport-of-pr-234…
Browse files Browse the repository at this point in the history
…4-on-v3.6.x

Backport PR #2344 on branch v3.6.x (multiselect support for dropdown default)
  • Loading branch information
kecnry authored Aug 25, 2023
2 parents 99d040e + 0398c83 commit 340c7c8
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ Bug Fixes
- Explot Plot now throws exception if its "save_figure" method is called
with a path that contains invalid directory. [#2339]

- Plugin dropdown elements with multiselect mode enabled will no longer reset
the selection when the choices change if any of the previous entries are still
valid. [#2344]

Cubeviz
^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion jdaviz/components/plugin_dataset_select.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div>
<v-row v-if="items.length > 1 || show_if_single_entry">
<v-row v-if="items.length > 1 || selected.length===0 || show_if_single_entry">
<v-select
:menu-props="{ left: true }"
attach
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/components/plugin_layer_select.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div>
<v-row v-if="items.length > 1 || show_if_single_entry">
<v-row v-if="items.length > 1 || selected.length===0 || show_if_single_entry">
<v-select
:menu-props="{ left: true }"
attach
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/components/plugin_subset_select.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div>
<v-row v-if="items.length > 1 || show_if_single_entry">
<v-row v-if="items.length > 1 || selected.length===0 || show_if_single_entry">
<v-select
:menu-props="{ left: true }"
attach
Expand Down
2 changes: 1 addition & 1 deletion jdaviz/components/plugin_viewer_select.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div>
<v-row v-if="items.length > 1 || show_if_single_entry">
<v-row v-if="items.length > 1 || selected.length===0 || show_if_single_entry">
<v-select
:menu-props="{ left: true }"
attach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ def test_multiselect(cubeviz_helper, spectrum1d_cube):
assert po.layer.multiselect is True

po.viewer.selected = ['flux-viewer', 'uncert-viewer']
# any internal call to reset the defaults (due to a change in the choices by adding or removing
# a viewer, etc) should not reset the selection
po.viewer._apply_default_selection(skip_if_current_valid=True)
assert po.viewer.selected == ['flux-viewer', 'uncert-viewer']

# from API could call po.axes_visible.value = False, but we'll also test the vue-level wrapper
po.vue_set_value({'name': 'axes_visible_value', 'value': False})
assert po.axes_visible.value is False
Expand Down
6 changes: 6 additions & 0 deletions jdaviz/configs/imviz/tests/test_viewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ def test_create_destroy_viewer(imviz_helper, desired_name, actual_name):
# Make sure plugins that store viewer_items are updated.
assert sorted(imviz_helper.plugins['Compass'].viewer.labels) == viewer_names

po = imviz_helper.plugins['Plot Options']
po.multiselect = True
po.viewer = viewer_names

imviz_helper.destroy_viewer(actual_name)
assert imviz_helper.app.get_viewer_ids() == ['imviz-0']
assert po.viewer.selected == ['imviz-0']
assert po.viewer.labels == ['imviz-0']


def test_get_viewer_created(imviz_helper):
Expand Down
13 changes: 12 additions & 1 deletion jdaviz/core/template_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,18 @@ def default_mode(self):
return self._default_mode

def _apply_default_selection(self, skip_if_current_valid=True):
# TODO: make this multi-ready
if self.is_multiselect:
if skip_if_current_valid and len(self.selected) == 0:
# current selection is empty and so should remain that way
return
is_valid = [s in self.labels for s in self.selected]
if skip_if_current_valid and np.any(is_valid):
if np.all(is_valid):
return
self.selected = [s for s in self.labels if s in self.selected]
return
is_valid = False

is_valid = self.selected in self.labels
if callable(self.default_mode):
# callable was defined and passed by the plugin or inheriting component.
Expand Down

0 comments on commit 340c7c8

Please sign in to comment.