Update card collapsed state handling to change child view model visibility attribute (#8274)

Author: dalthviz

panel/models/card.ts

@@ -4,6 +4,7 @@ import {Container} from "@bokehjs/core/layout/grid"
 import type * as p from "@bokehjs/core/properties"
 import {GridAlignmentLayout} from "@bokehjs/models/layouts/alignments"
 import {LayoutDOMView} from "@bokehjs/models/layouts/layout_dom"
+import type {UIElementView} from "@bokehjs/models/ui/ui_element"
 
 import {Column, ColumnView} from "./column"
 
@@ -22,6 +23,9 @@ export class CardView extends ColumnView {
 
   button_el: HTMLButtonElement
   header_el: HTMLElement
+  visible_child_views: Map<UIElementView, boolean> = new Map()
+  protected _updating_child_visibility: Set<UIElementView> = new Set()
+  protected _child_visible_callbacks: Map<UIElementView, () => void> = new Map()
 
   readonly collapsed_style = new DOM.InlineStyleSheet()
 
@@ -40,6 +44,10 @@ export class CardView extends ColumnView {
       this.child_views[0].el.style.backgroundColor = header_background
       this.header_el.style.backgroundColor = header_background
     })
+
+    for (const child_view of this.child_views.slice(1)) {
+      this._register_child_view(child_view)
+    }
   }
 
   override stylesheets(): StyleSheetLike[] {
@@ -106,11 +114,9 @@ export class CardView extends ColumnView {
       header.r_after_render()
     }
 
-    if (this.model.collapsed) {
-      return
-    }
-
     for (const child_view of this.child_views.slice(1)) {
+      this._register_child_view(child_view)
+      this._apply_child_visible(child_view)
       this.shadow_el.appendChild(child_view.el)
       child_view.render()
       child_view.r_after_render()
@@ -119,6 +125,15 @@ export class CardView extends ColumnView {
 
   override async update_children(): Promise<void> {
     await this.build_child_views()
+    const child_views = new Set(this.child_views.slice(1))
+    for (const child_view of this.visible_child_views.keys()) {
+      if (!child_views.has(child_view)) {
+        this._unregister_child_view(child_view)
+      }
+    }
+    for (const child_view of child_views) {
+      this._register_child_view(child_view)
+    }
     this.render()
     this.invalidate_layout()
   }
@@ -210,14 +225,15 @@ export class CardView extends ColumnView {
 
   _collapse(): void {
     for (const child_view of this.child_views.slice(1)) {
+      this._register_child_view(child_view)
       if (this.model.collapsed) {
         this.shadow_el.removeChild(child_view.el)
-        child_view.model.visible = false
+        this._set_child_visible(child_view, false)
       } else {
         child_view.render()
         child_view.after_render()
         this.shadow_el.appendChild(child_view.el)
-        child_view.model.visible = true
+        this._apply_child_visible(child_view)
       }
     }
     if (this.model.collapsed) {
@@ -232,6 +248,54 @@ export class CardView extends ColumnView {
     this.invalidate_layout()
   }
 
+  protected _set_child_visible(child_view: UIElementView, visible: boolean): void {
+    if (child_view.model.visible == visible) {
+      return
+    }
+    this._updating_child_visibility.add(child_view)
+    try {
+      child_view.model.visible = visible
+    } finally {
+      this._updating_child_visibility.delete(child_view)
+    }
+  }
+
+  protected _apply_child_visible(child_view: UIElementView): void {
+    const desired_visible = this.visible_child_views.get(child_view) ?? child_view.model.visible
+    this.visible_child_views.set(child_view, desired_visible)
+    this._set_child_visible(child_view, desired_visible && !this.model.collapsed)
+  }
+
+  protected _register_child_view(child_view: UIElementView): void {
+    if (this.visible_child_views.has(child_view)) {
+      return
+    }
+    this.visible_child_views.set(child_view, child_view.model.visible)
+    const {visible} = child_view.model.properties
+    const callback = () => {
+      if (this._updating_child_visibility.has(child_view)) {
+        return
+      }
+      const desired_visible = child_view.model.visible
+      this.visible_child_views.set(child_view, desired_visible)
+      if (this.model.collapsed && desired_visible) {
+        this._set_child_visible(child_view, false)
+      }
+    }
+    this._child_visible_callbacks.set(child_view, callback)
+    this.on_change(visible, callback)
+  }
+
+  protected _unregister_child_view(child_view: UIElementView): void {
+    const callback = this._child_visible_callbacks.get(child_view)
+    if (callback != null) {
+      child_view.model.properties.visible.change.disconnect(callback)
+      this._child_visible_callbacks.delete(child_view)
+    }
+    this.visible_child_views.delete(child_view)
+    this._updating_child_visibility.delete(child_view)
+  }
+
   protected override _create_element(): HTMLElement {
     return DOM.create_element((this.model.tag as any), {class: this.css_classes()})
   }

panel/tests/test_server.py

@@ -839,6 +839,7 @@ def cb(event):
     wait_until(lambda: bool(called))
 
 
+@pytest.mark.flaky(max_runs=3)
 def test_server_thread_pool_change_event(server_implementation, threads):
     button = Button(name='Click')
     button2 = Button(name='Click')

panel/tests/ui/layout/test_accordion.py

@@ -19,9 +19,9 @@ def accordion_components():
     return d0, d1
 
 
-def is_collapsed(card_object, card_content):
+def is_collapsed(card_object, card_content, page):
     expect(card_object.locator('svg')).to_have_class("icon icon-tabler icons-tabler-outline icon-tabler-chevron-right")
-    expect(card_object).not_to_contain_text(card_content)
+    expect(page.get_by_text(card_content)).to_be_hidden()
     return True
 
 
@@ -51,8 +51,8 @@ def test_accordion_default(page, accordion_components):
     expect(d1_object).to_contain_text(d1.name)
 
     # cards are collapsed and their content is hidden by default
-    assert is_collapsed(card_object=d0_object, card_content=d0.text)
-    assert is_collapsed(card_object=d1_object, card_content=d1.text)
+    assert is_collapsed(card_object=d0_object, card_content=d0.text, page=page)
+    assert is_collapsed(card_object=d1_object, card_content=d1.text, page=page)
 
     # cards can be expanded simultaneously
     d0_object.click()
@@ -94,7 +94,7 @@ def test_accordion_active(page, accordion_components):
     # second card is collapsed and its content is hidden
     d1_object = accordion_elements.nth(1)
     d1_object.wait_for()
-    assert is_collapsed(card_object=d1_object, card_content=d1.text)
+    assert is_collapsed(card_object=d1_object, card_content=d1.text, page=page)
 
 
 def test_accordion_objects(page, accordion_components):
@@ -127,11 +127,11 @@ def test_accordion_toggle(page, accordion_components):
     # click to expand the first card, 2nd card is collapsed
     d0_object.click()
     assert is_expanded(card_object=d0_object, card_content=d0.text)
-    assert is_collapsed(card_object=d1_object, card_content=d1.text)
+    assert is_collapsed(card_object=d1_object, card_content=d1.text, page=page)
 
     # click to expand the 2nd card, 1st card is collapsed
     d1_object.click()
-    assert is_collapsed(card_object=d0_object, card_content=d0.text)
+    assert is_collapsed(card_object=d0_object, card_content=d0.text, page=page)
     assert is_expanded(card_object=d1_object, card_content=d1.text)
 
 
@@ -150,15 +150,15 @@ def test_accordion_append(page, accordion_components):
     expect(accordion_elements).to_have_count(1)
     # collapsed by default
     d0_object = accordion_elements.nth(0)
-    assert is_collapsed(card_object=d0_object, card_content=d0.text)
+    assert is_collapsed(card_object=d0_object, card_content=d0.text, page=page)
 
     # add new element d1
     accordion.append(d1)
     expect(accordion_elements).to_have_count(2)
     d1_object = accordion_elements.nth(1)
     # both cards are collapsed
-    assert is_collapsed(card_object=d0_object, card_content=d0.text)
-    assert is_collapsed(card_object=d1_object, card_content=d1.text)
+    assert is_collapsed(card_object=d0_object, card_content=d0.text, page=page)
+    assert is_collapsed(card_object=d1_object, card_content=d1.text, page=page)
 
 
 def test_accordion_extend(page, accordion_components):
@@ -179,9 +179,9 @@ def test_accordion_extend(page, accordion_components):
     d0_object = accordion_elements.nth(0)
     d1_object = accordion_elements.nth(1)
     d2_object = accordion_elements.nth(2)
-    assert is_collapsed(card_object=d0_object, card_content=d0.text)
-    assert is_collapsed(card_object=d1_object, card_content=d1.text)
-    assert is_collapsed(card_object=d2_object, card_content=d2.text)
+    assert is_collapsed(card_object=d0_object, card_content=d0.text, page=page)
+    assert is_collapsed(card_object=d1_object, card_content=d1.text, page=page)
+    assert is_collapsed(card_object=d2_object, card_content=d2.text, page=page)
 
 
 def test_accordion_clear(page, accordion_components):

panel/tests/ui/layout/test_card.py

@@ -201,3 +201,44 @@ def test_card_widget_not_collapsed(page, card_components):
 
     wait_until(lambda: w2.value == 'F', page)
     assert not card.collapsed
+
+
+def test_card_child_visible(page, card_components):
+    w1, w2 = card_components
+    w1.visible = False
+    card = Card(w1, title="Foo", collapsed=True)
+
+    serve_component(page, card)
+
+    slider = page.locator('.bk-Slider')
+    expect(slider).not_to_be_visible()
+
+    w1.visible = True
+
+    expect(slider).not_to_be_visible()
+
+    card.collapsed = False
+
+    expect(slider).to_be_visible()
+
+    card.collapsed = True
+
+    # Ensure newly added component respects visibility
+    card.append(w2)
+
+    text_input = page.locator(".class_w2")
+
+    expect(text_input).not_to_be_visible()
+
+    # Ensure opening card renders visible component
+    card.collapsed = False
+    expect(text_input).to_be_visible()
+
+    # Ensure setting visible restores visibility
+    w2.visible = False
+    expect(text_input).not_to_be_visible()
+
+    # Ensure toggling collapsed doesn't override visible state
+    card.collapsed = True
+    card.collapsed = False
+    expect(text_input).not_to_be_visible()

panel/tests/ui/widgets/test_texteditor.py

@@ -62,6 +62,7 @@ def test_texteditor_enter_value(page):
     wait_until(lambda: widget.value == '<p>test</p>', page)
 
 
+@pytest.mark.flaky(max_runs=3)
 def test_texteditor_regression_copy_paste(page, browser):
     # https://github.com/holoviz/panel/issues/5545
     widget = TextEditor()