From ed583f3356f374d473dfc6fcc6509102eecea2ee Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Tue, 23 Apr 2024 14:09:06 -0700 Subject: [PATCH 1/8] clean up popup --- holoviews/plotting/bokeh/callbacks.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 02a5d8ccab..26117c9d23 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -670,6 +670,14 @@ def on_msg(self, msg): if hasattr(self, '_panel'): self._process_selection_event() + def _reposition(self, position): + if position: + self._panel.visible = True + self._panel.position = XY(**position) + self._existing_popup.visible = True + if self.plot.comm: + push_on_root(self.plot.root.ref['id']) + def _process_selection_event(self): event = self._selection_event if event is not None: @@ -716,16 +724,11 @@ def _process_selection_event(self): else: self._panel.stylesheets = [] - 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 if self._existing_popup and not self._existing_popup.visible: - if position: - self._panel.position = XY(**position) - self._existing_popup.visible = True - if self.plot.comm: - push_on_root(self.plot.root.ref['id']) + self._reposition(position) return model = popup_pane.get_root(self.plot.document, self.plot.comm) @@ -740,10 +743,8 @@ def _process_selection_event(self): )) # the first element is the close button self._panel.elements = [self._panel.elements[0], model] - if self.plot.comm: - push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane - + self._reposition(position) class TapCallback(PopupMixin, PointerXYCallback): """ From 19ad74c907ac5aa9b0f6374b4cf3576dd0b49d82 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Tue, 23 Apr 2024 15:06:15 -0700 Subject: [PATCH 2/8] fixes --- holoviews/plotting/bokeh/callbacks.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 26117c9d23..c201e16067 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -670,11 +670,11 @@ def on_msg(self, msg): if hasattr(self, '_panel'): self._process_selection_event() - def _reposition(self, position): + def _reposition_panel(self, position): if position: + self._existing_popup.visible = True self._panel.visible = True self._panel.position = XY(**position) - self._existing_popup.visible = True if self.plot.comm: push_on_root(self.plot.root.ref['id']) @@ -711,6 +711,9 @@ def _process_selection_event(self): position = None popup_pane = panel(popup) + if not popup_pane.visible: + return + if not popup_pane.stylesheets: self._panel.stylesheets = [ """ @@ -728,9 +731,12 @@ def _process_selection_event(self): # 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._reposition(position) + self._reposition_panel(position) return + if popup_pane.visible: + popup_pane.visible = True + model = popup_pane.get_root(self.plot.document, self.plot.comm) model.js_on_change('visible', CustomJS( args=dict(panel=self._panel), @@ -744,7 +750,7 @@ def _process_selection_event(self): # the first element is the close button self._panel.elements = [self._panel.elements[0], model] self._existing_popup = popup_pane - self._reposition(position) + self._reposition_panel(position) class TapCallback(PopupMixin, PointerXYCallback): """ From a23694fb4a5efd131f18be67ed714a0493ec1b3b Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Tue, 23 Apr 2024 15:09:44 -0700 Subject: [PATCH 3/8] more cleanup --- holoviews/plotting/bokeh/callbacks.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index c201e16067..2d15f6aaf4 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -672,7 +672,6 @@ def on_msg(self, msg): def _reposition_panel(self, position): if position: - self._existing_popup.visible = True self._panel.visible = True self._panel.position = XY(**position) if self.plot.comm: @@ -701,8 +700,6 @@ def _process_selection_event(self): if popup is None: if self._panel.visible: self._panel.visible = False - if self._existing_popup and not self._existing_popup.visible: - self._existing_popup.visible = False return if event is not None: @@ -734,9 +731,6 @@ def _process_selection_event(self): self._reposition_panel(position) return - if popup_pane.visible: - popup_pane.visible = True - model = popup_pane.get_root(self.plot.document, self.plot.comm) model.js_on_change('visible', CustomJS( args=dict(panel=self._panel), From 0dab63d2fa1d4577c8aea2d40b5e248389c2a2f9 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Tue, 23 Apr 2024 15:19:10 -0700 Subject: [PATCH 4/8] revamp --- holoviews/plotting/bokeh/callbacks.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 2d15f6aaf4..16d7b6356e 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -670,13 +670,6 @@ def on_msg(self, msg): if hasattr(self, '_panel'): self._process_selection_event() - def _reposition_panel(self, position): - if position: - self._panel.visible = True - self._panel.position = XY(**position) - if self.plot.comm: - push_on_root(self.plot.root.ref['id']) - def _process_selection_event(self): event = self._selection_event if event is not None: @@ -707,7 +700,6 @@ def _process_selection_event(self): else: position = None popup_pane = panel(popup) - if not popup_pane.visible: return @@ -724,11 +716,15 @@ def _process_selection_event(self): else: self._panel.stylesheets = [] + 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 if self._existing_popup and not self._existing_popup.visible: - self._reposition_panel(position) + if position: + if self.plot.comm: + push_on_root(self.plot.root.ref['id']) + self._panel.position = XY(**position) return model = popup_pane.get_root(self.plot.document, self.plot.comm) @@ -743,8 +739,10 @@ def _process_selection_event(self): )) # the first element is the close button self._panel.elements = [self._panel.elements[0], model] + if self.plot.comm: + push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane - self._reposition_panel(position) + class TapCallback(PopupMixin, PointerXYCallback): """ From 60acb16436f6b897acb8d859f1f0abbfbec442a8 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Wed, 24 Apr 2024 07:08:44 -0700 Subject: [PATCH 5/8] revert change --- holoviews/plotting/bokeh/callbacks.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 16d7b6356e..9f68829fec 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -722,9 +722,9 @@ def _process_selection_event(self): # meaning the popup has already been removed; we need to regenerate if self._existing_popup and not self._existing_popup.visible: if position: - if self.plot.comm: - push_on_root(self.plot.root.ref['id']) self._panel.position = XY(**position) + if self.plot.comm: # update on notebook + push_on_root(self.plot.root.ref['id']) return model = popup_pane.get_root(self.plot.document, self.plot.comm) @@ -739,7 +739,7 @@ def _process_selection_event(self): )) # the first element is the close button self._panel.elements = [self._panel.elements[0], model] - if self.plot.comm: + if self.plot.comm: # update on notebook push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane From 3897f8c83ad1ee588acfa1a9f5afeb33598a3245 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Wed, 24 Apr 2024 07:43:28 -0700 Subject: [PATCH 6/8] Simply set invisible --- holoviews/plotting/bokeh/callbacks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/holoviews/plotting/bokeh/callbacks.py b/holoviews/plotting/bokeh/callbacks.py index 9f68829fec..4d50c7537c 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -699,6 +699,7 @@ def _process_selection_event(self): position = self._get_position(event) else: position = None + popup_pane = panel(popup) if not popup_pane.visible: return @@ -733,7 +734,7 @@ def _process_selection_event(self): code=""" export default ({panel}, event, _) => { if (!event.visible) { - panel.position.setv({x: NaN, y: NaN}) + panel.visible = false; } }""", )) From 0c73d020a0543ca9a712a84b168c34d9b1ce8b79 Mon Sep 17 00:00:00 2001 From: Andrew Huang Date: Wed, 24 Apr 2024 08:01:09 -0700 Subject: [PATCH 7/8] fix test --- holoviews/tests/ui/bokeh/test_callback.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/holoviews/tests/ui/bokeh/test_callback.py b/holoviews/tests/ui/bokeh/test_callback.py index b1cb47d4d4..93eb11de3a 100644 --- a/holoviews/tests/ui/bokeh/test_callback.py +++ b/holoviews/tests/ui/bokeh/test_callback.py @@ -284,12 +284,13 @@ def hide(_): # initial appearance locator = page.locator(".bk-btn") expect(locator).to_have_count(2) + expect(locator.first).to_be_visible() # click custom button to hide locator = page.locator(".custom-button") locator.click() locator = page.locator(".bk-btn") - expect(locator).to_have_count(0) + expect(locator.first).not_to_be_visible() From 40d59e05c9834b6ee4e36d512bf671180769ec9f Mon Sep 17 00:00:00 2001 From: Andrew <15331990+ahuang11@users.noreply.github.com> Date: Mon, 6 May 2024 08:54:45 -0700 Subject: [PATCH 8/8] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Simon Høxbro Hansen --- 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 4d50c7537c..82698dbf73 100644 --- a/holoviews/plotting/bokeh/callbacks.py +++ b/holoviews/plotting/bokeh/callbacks.py @@ -724,7 +724,7 @@ def _process_selection_event(self): if self._existing_popup and not self._existing_popup.visible: if position: self._panel.position = XY(**position) - if self.plot.comm: # update on notebook + if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) return @@ -740,7 +740,7 @@ def _process_selection_event(self): )) # the first element is the close button self._panel.elements = [self._panel.elements[0], model] - if self.plot.comm: # update on notebook + if self.plot.comm: # update Jupyter Notebook push_on_root(self.plot.root.ref['id']) self._existing_popup = popup_pane