From 30e690a61f6930fee30197a6b67e0d6acc0b71a1 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 10:38:01 -0700 Subject: [PATCH 1/6] rename for readability, apply fixes, and move unnecessary calc --- holoviews/plotting/bokeh/callbacks.py | 44 ++++++++++++++------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 0b4b95a3de..e785a3559a 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -609,23 +609,23 @@ def initialize(self, plot_id=None): } """], css_classes=["popup-close-btn"]) - self._panel = Panel( + self._bk_panel = Panel( position=XY(x=np.nan, y=np.nan), anchor="top_left", elements=[close_button], visible=False, styles={"zIndex": "1000"}, ) - close_button.js_on_click(CustomJS(args=dict(panel=self._panel), code="panel.visible = false")) + close_button.js_on_click(CustomJS(args=dict(panel=self._bk_panel), code="panel.visible = false")) - self.plot.state.elements.append(self._panel) + self.plot.state.elements.append(self._bk_panel) self._watch_position() def _watch_position(self): geom_type = self.geom_type self.plot.state.on_event('selectiongeometry', self._update_selection_event) self.plot.state.js_on_event('selectiongeometry', CustomJS( - args=dict(panel=self._panel), + args=dict(panel=self._bk_panel), code=f""" export default ({{panel}}, cb_obj, _) => {{ const el = panel.elements[1] @@ -668,7 +668,7 @@ def _update_selection_event(self, event): def on_msg(self, msg): super().on_msg(msg) - if hasattr(self, '_panel'): + if hasattr(self, '_bk_panel'): self._process_selection_event() def _process_selection_event(self): @@ -687,27 +687,27 @@ def _process_selection_event(self): if popup is not None: break + popup_is_callable = False if callable(popup): + popup_is_callable = True with set_curdoc(self.plot.document): popup = popup(**stream.contents) - # If no popup is defined, hide the panel + # If no popup is defined, hide the bokeh panel wrapper if popup is None: - if self._panel.visible: - self._panel.visible = False + if self._bk_panel.visible: + self._bk_panel.visible = False return - if event is not None: - position = self._get_position(event) - else: - position = None - popup_pane = panel(popup) - if not popup_pane.visible: + # offer the user ability to control when the popup bk panel shows up + # however, if the popup is not callable, we will have to assume + # it's always visible else if they make it invisible, it'll never re-appear + if popup_is_callable and not popup_pane.visible: return if not popup_pane.stylesheets: - self._panel.stylesheets = [ + self._bk_panel.stylesheets = [ """ :host { padding: 1em; @@ -717,22 +717,24 @@ def _process_selection_event(self): """, ] else: - self._panel.stylesheets = [] + self._bk_panel.stylesheets = [] - self._panel.visible = True + self._bk_panel.visible = True # for existing popup, important to check if they're visible # otherwise, UnknownReferenceError: can't resolve reference 'p...' # meaning the popup has already been removed; we need to regenerate if self._existing_popup and not self._existing_popup.visible: + self._existing_popup.visible = True + position = self._get_position(event) if event else None if position: - self._panel.position = XY(**position) + self._bk_panel.position = XY(**position) if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) return model = popup_pane.get_root(self.plot.document, self.plot.comm) model.js_on_change('visible', CustomJS( - args=dict(panel=self._panel), + args=dict(panel=self._bk_panel), code=""" export default ({panel}, event, _) => { if (!event.visible) { @@ -741,7 +743,7 @@ def _process_selection_event(self): }""", )) # the first element is the close button - self._panel.elements = [self._panel.elements[0], model] + self._bk_panel.elements = [self._bk_panel.elements[0], model] if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane @@ -1114,7 +1116,7 @@ def _watch_position(self): renderer = self.plot.handles['glyph_renderer'] selected = self.plot.handles['selected'] self.plot.state.js_on_event('selectiongeometry', CustomJS( - args=dict(panel=self._panel, renderer=renderer, source=source, selected=selected), + args=dict(panel=self._bk_panel, renderer=renderer, source=source, selected=selected), code=""" export default ({panel, renderer, source, selected}, cb_obj, _) => { const el = panel.elements[1] From 03b82b6007029d5e4723a2a5e619b0dc7c8dc5f8 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 10:43:43 -0700 Subject: [PATCH 2/6] revert a change --- holoviews/plotting/bokeh/callbacks.py | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index e785a3559a..9b5ea260e1 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -609,23 +609,23 @@ def initialize(self, plot_id=None): } """], css_classes=["popup-close-btn"]) - self._bk_panel = Panel( + self._panel = Panel( position=XY(x=np.nan, y=np.nan), anchor="top_left", elements=[close_button], visible=False, styles={"zIndex": "1000"}, ) - close_button.js_on_click(CustomJS(args=dict(panel=self._bk_panel), code="panel.visible = false")) + close_button.js_on_click(CustomJS(args=dict(panel=self._panel), code="panel.visible = false")) - self.plot.state.elements.append(self._bk_panel) + self.plot.state.elements.append(self._panel) self._watch_position() def _watch_position(self): geom_type = self.geom_type self.plot.state.on_event('selectiongeometry', self._update_selection_event) self.plot.state.js_on_event('selectiongeometry', CustomJS( - args=dict(panel=self._bk_panel), + args=dict(panel=self._panel), code=f""" export default ({{panel}}, cb_obj, _) => {{ const el = panel.elements[1] @@ -668,7 +668,7 @@ def _update_selection_event(self, event): def on_msg(self, msg): super().on_msg(msg) - if hasattr(self, '_bk_panel'): + if hasattr(self, '_panel'): self._process_selection_event() def _process_selection_event(self): @@ -695,8 +695,8 @@ def _process_selection_event(self): # If no popup is defined, hide the bokeh panel wrapper if popup is None: - if self._bk_panel.visible: - self._bk_panel.visible = False + if self._panel.visible: + self._panel.visible = False return popup_pane = panel(popup) @@ -707,7 +707,7 @@ def _process_selection_event(self): return if not popup_pane.stylesheets: - self._bk_panel.stylesheets = [ + self._panel.stylesheets = [ """ :host { padding: 1em; @@ -717,9 +717,9 @@ def _process_selection_event(self): """, ] else: - self._bk_panel.stylesheets = [] + self._panel.stylesheets = [] - self._bk_panel.visible = True + self._panel.visible = True # for existing popup, important to check if they're visible # otherwise, UnknownReferenceError: can't resolve reference 'p...' # meaning the popup has already been removed; we need to regenerate @@ -727,14 +727,14 @@ def _process_selection_event(self): self._existing_popup.visible = True position = self._get_position(event) if event else None if position: - self._bk_panel.position = XY(**position) + self._panel.position = XY(**position) if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) return model = popup_pane.get_root(self.plot.document, self.plot.comm) model.js_on_change('visible', CustomJS( - args=dict(panel=self._bk_panel), + args=dict(panel=self._panel), code=""" export default ({panel}, event, _) => { if (!event.visible) { @@ -743,7 +743,7 @@ def _process_selection_event(self): }""", )) # the first element is the close button - self._bk_panel.elements = [self._bk_panel.elements[0], model] + self._panel.elements = [self._panel.elements[0], model] if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane @@ -1116,7 +1116,7 @@ def _watch_position(self): renderer = self.plot.handles['glyph_renderer'] selected = self.plot.handles['selected'] self.plot.state.js_on_event('selectiongeometry', CustomJS( - args=dict(panel=self._bk_panel, renderer=renderer, source=source, selected=selected), + args=dict(panel=self._panel, renderer=renderer, source=source, selected=selected), code=""" export default ({panel, renderer, source, selected}, cb_obj, _) => { const el = panel.elements[1] From 2d286d41b939884d8e7d8b16c3fd472b7ab0c2d5 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 10:50:46 -0700 Subject: [PATCH 3/6] simplify --- holoviews/plotting/bokeh/callbacks.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 9b5ea260e1..3821acdcd7 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -687,9 +687,8 @@ def _process_selection_event(self): if popup is not None: break - popup_is_callable = False - if callable(popup): - popup_is_callable = True + popup_is_callable = callable(popup) + if popup_is_callable: with set_curdoc(self.plot.document): popup = popup(**stream.contents) From 8b7739d13b976f999f60ca48ab4a48338ea219bb Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 10:53:37 -0700 Subject: [PATCH 4/6] clarify --- holoviews/plotting/bokeh/callbacks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 3821acdcd7..5e6011aafc 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -700,8 +700,8 @@ def _process_selection_event(self): popup_pane = panel(popup) # offer the user ability to control when the popup bk panel shows up - # however, if the popup is not callable, we will have to assume - # it's always visible else if they make it invisible, it'll never re-appear + # however, if the popup is not callable (singleton), we cannot do this + # check--else, it'll never re-appear if they set `visible=False` once if popup_is_callable and not popup_pane.visible: return From b2a8cb101e6efed32646d2af0af4f650380bf981 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 10:59:11 -0700 Subject: [PATCH 5/6] add couple tests --- holoviews/tests/ui/bokeh/test_callback.py | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/holoviews/tests/ui/bokeh/test_callback.py b/holoviews/tests/ui/bokeh/test_callback.py index 395dec132f..dcf3e1a25f 100644 --- a/holoviews/tests/ui/bokeh/test_callback.py +++ b/holoviews/tests/ui/bokeh/test_callback.py @@ -315,6 +315,11 @@ def popup_form(x, y): page.click(".bk-btn.bk-btn-default") expect(locator).not_to_be_visible() + hv_plot.click() + locator = page.locator(".bk-btn.bk-btn-default") + expect(locator).to_have_count(1) + expect(locator).to_be_visible() + @skip_popup @pytest.mark.usefixtures("bokeh_backend") @@ -346,6 +351,47 @@ def popup_form(index): expect(locator).to_have_count(1) +@skip_popup +@pytest.mark.usefixtures("bokeh_backend") +def test_stream_popup_noncallable_reappear(serve_hv, points): + def popup_form(name): + text_input = pn.widgets.TextInput(name='Description') + button = pn.widgets.Button( + name='Save', + on_click=lambda _: layout.param.update(visible=False), + button_type="primary" + ) + layout = pn.Column(f'# {name}', text_input, button) + return layout + + points = points.opts(hit_dilation=5) + hv.streams.Tap(source=points, popup=popup_form('Tap')) + points.opts(tools=["tap"], active_tools=["tap"]) + + page = serve_hv(points) + hv_plot = page.locator('.bk-events') + expect(hv_plot).to_have_count(1) + + hv_plot.click() + + locator = page.locator("#tap") + expect(locator).to_have_count(1) + locator = page.locator(".bk-btn bk-btn-primary") + expect(locator).to_have_count(1) + expect(locator).to_be_visible() + + page.click(".bk-btn bk-btn-primary") + expect(locator).not_to_be_visible() + + hv_plot.click() + + locator = page.locator("#tap") + expect(locator).to_have_count(1) + locator = page.locator(".bk-btn bk-btn-primary") + expect(locator).to_have_count(1) + expect(locator).to_be_visible() + + @skip_popup @pytest.mark.usefixtures("bokeh_backend") def test_stream_popup_selection1d_lasso_select(serve_hv, points): From 9fea81c5799820c3f33db83c1586763457af5db4 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Mon, 29 Jul 2024 11:01:22 -0700 Subject: [PATCH 6/6] fix class test --- holoviews/tests/ui/bokeh/test_callback.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/holoviews/tests/ui/bokeh/test_callback.py b/holoviews/tests/ui/bokeh/test_callback.py index dcf3e1a25f..725df28698 100644 --- a/holoviews/tests/ui/bokeh/test_callback.py +++ b/holoviews/tests/ui/bokeh/test_callback.py @@ -376,18 +376,18 @@ def popup_form(name): locator = page.locator("#tap") expect(locator).to_have_count(1) - locator = page.locator(".bk-btn bk-btn-primary") + locator = page.locator(".bk-btn.bk-btn-primary") expect(locator).to_have_count(1) expect(locator).to_be_visible() - page.click(".bk-btn bk-btn-primary") + page.click(".bk-btn.bk-btn-primary") expect(locator).not_to_be_visible() hv_plot.click() locator = page.locator("#tap") expect(locator).to_have_count(1) - locator = page.locator(".bk-btn bk-btn-primary") + locator = page.locator(".bk-btn.bk-btn-primary") expect(locator).to_have_count(1) expect(locator).to_be_visible()