Skip to content

Commit 6b8a6e3

Browse files
committed
[DREAM-808] Fixes removed checked node form state
https://community.openproject.org/wp/DREAM-808 Extends the childList guard to inspect removed nodes symmetrically with added ones, so removing a checked tree item also rebuilds the hidden form inputs instead of leaving a stale input that submits the deleted node's value. Extracts the shared check into a private helper. Also passes attributeOldValue: true to the observer so the existing oldValue comparison actually filters out same-value aria-checked rewrites instead of rebuilding the inputs on every write. Adds a browser regression test removing a checked node from a connected tree, and a changeset for the insertion and removal fixes.
1 parent 46506c1 commit 6b8a6e3

3 files changed

Lines changed: 39 additions & 8 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@openproject/primer-view-components': patch
3+
---
4+
5+
Fix TreeView hidden form inputs falling out of sync when checked nodes are inserted or removed after the component connects.

app/components/primer/alpha/tree_view/tree_view.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,7 @@ export class TreeViewElement extends HTMLElement {
3939
// looping, so we make sure something actually changed before computing inputs again.
4040
const somethingChanged = mutations.some(m => {
4141
if (m.type === 'childList') {
42-
return [...m.addedNodes].some(node => {
43-
if (!(node instanceof Element)) return false
44-
45-
return (
46-
node.matches('[role=treeitem][aria-checked=true]') ||
47-
Boolean(node.querySelector('[role=treeitem][aria-checked=true]'))
48-
)
49-
})
42+
return this.#containsCheckedTreeItem(m.addedNodes) || this.#containsCheckedTreeItem(m.removedNodes)
5043
}
5144

5245
if (!(m.target instanceof HTMLElement)) return false
@@ -62,6 +55,7 @@ export class TreeViewElement extends HTMLElement {
6255
childList: true,
6356
subtree: true,
6457
attributeFilter: ['aria-checked'],
58+
attributeOldValue: true,
6559
})
6660

6761
// Correctly initialize the form
@@ -76,6 +70,17 @@ export class TreeViewElement extends HTMLElement {
7670
})
7771
}
7872

73+
#containsCheckedTreeItem(nodes: NodeList): boolean {
74+
return [...nodes].some(node => {
75+
if (!(node instanceof Element)) return false
76+
77+
return (
78+
node.matches('[role=treeitem][aria-checked=true]') ||
79+
Boolean(node.querySelector('[role=treeitem][aria-checked=true]'))
80+
)
81+
})
82+
}
83+
7984
rootLeafNodes(): NodeListOf<HTMLElement> {
8085
return this.querySelectorAll(':scope > ul > li > .TreeViewItemContainer [role=treeitem]')
8186
}

test/system/alpha/tree_view_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,27 @@ def test_form_state_updates_when_checked_nodes_are_inserted
978978
assert_includes response.dig("form_params", "folder_structure"), "{\"path\":[\"async.rb\"]}"
979979
end
980980

981+
def test_form_state_updates_when_checked_nodes_are_removed
982+
visit_preview(:form_input, expanded: true, route_format: :json)
983+
984+
assert_selector("[data-target='tree-view.formInputContainer'] input", count: 1, visible: :all)
985+
986+
evaluate_multiline_script(<<~JS)
987+
const tree = document.querySelector("tree-view")
988+
const checkedNode = tree.querySelector("[role=treeitem][aria-checked=true]")
989+
990+
checkedNode.closest("li").remove()
991+
JS
992+
993+
assert_selector("[data-target='tree-view.formInputContainer'] input", count: 0, visible: :all)
994+
995+
find("button[type=submit]").click
996+
997+
response = JSON.parse(find("pre").text)
998+
999+
assert_nil response.dig("form_params", "folder_structure")
1000+
end
1001+
9811002
def test_form_submission_with_single_select_variant
9821003
visit_preview(:form_input, expanded: true, select_variant: :single, route_format: :json)
9831004

0 commit comments

Comments
 (0)