Skip to content

Commit efe0bb5

Browse files
committed
TreeStore: Fix ARIA desync (siblingIndex and siblingCount) after sort and filter (#9431)
1 parent 438b63d commit efe0bb5

2 files changed

Lines changed: 174 additions & 25 deletions

File tree

src/data/TreeStore.mjs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,12 @@ class TreeStore extends Store {
404404
me.sortArray(children);
405405
}
406406

407-
// 3. Re-project the flat _items array from the sorted Structural Layer
407+
// 3. Update sibling stats after sorting to reflect new indices
408+
for (let parentId of me.#childrenMap.keys()) {
409+
me.updateSiblingStats(parentId);
410+
}
411+
412+
// 4. Re-project the flat _items array from the sorted Structural Layer
408413
me._items = [];
409414
let roots = me.#childrenMap.get('root') || [];
410415
for (let i = 0, len = roots.length; i < len; i++) {
@@ -731,6 +736,11 @@ class TreeStore extends Store {
731736

732737
me._keys = me._items.map(item => me.getKey(item));
733738

739+
// Update sibling stats to reflect the filtered counts and indices
740+
for (let parentId of me.#childrenMap.keys()) {
741+
me.updateSiblingStats(parentId);
742+
}
743+
734744
if (me[updatingIndex] === 0) {
735745
me.fire('filter', {
736746
isFiltered: me[isFilteredSymbol],
@@ -827,7 +837,13 @@ class TreeStore extends Store {
827837
siblings = me.#childrenMap.get(parentId);
828838

829839
if (siblings) {
830-
let count = siblings.length;
840+
// Determine visible siblings based on the active filter mask
841+
let visibleSiblings = siblings;
842+
if (me._keptNodes) {
843+
visibleSiblings = siblings.filter(node => me._keptNodes.has(me.getKey(node)));
844+
}
845+
846+
let count = visibleSiblings.length;
831847

832848
// 1. Update Parent's childCount
833849
if (parentId !== 'root') {
@@ -848,7 +864,7 @@ class TreeStore extends Store {
848864

849865
// 2. Update Siblings' ARIA stats
850866
for (let i = 0; i < count; i++) {
851-
let sibling = siblings[i];
867+
let sibling = visibleSiblings[i];
852868
if (sibling.siblingCount !== count || sibling.siblingIndex !== i + 1) {
853869
sibling.siblingCount = count;
854870
sibling.siblingIndex = i + 1; // 1-based for ARIA

test/playwright/unit/data/TreeStore.spec.mjs

Lines changed: 155 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -286,19 +286,17 @@ test.describe('Neo.data.TreeStore (Hierarchical Sorting)', () => {
286286
*/
287287
let store, TestModel;
288288

289-
test.beforeEach(() => {
290-
class TestTreeModel4 extends TreeModel {
291-
static config = {
292-
className: 'Test.Unit.Data.TreeStore.TestTreeModel4',
293-
fields: [
294-
{ name: 'id', type: 'String' },
295-
{ name: 'name', type: 'String' }
296-
]
297-
}
289+
class TestTreeModel4 extends TreeModel {
290+
static config = {
291+
className: 'Test.Unit.Data.TreeStore.TestTreeModel4',
292+
fields: [
293+
{ name: 'id', type: 'String' },
294+
{ name: 'name', type: 'String' }
295+
]
298296
}
297+
}
299298

300-
TestModel = Neo.setupClass(TestTreeModel4);
301-
});
299+
TestModel = Neo.setupClass(TestTreeModel4);
302300

303301
test.afterEach(() => {
304302
store?.destroy();
@@ -388,6 +386,77 @@ test.describe('Neo.data.TreeStore (Hierarchical Sorting)', () => {
388386
expect(items[3].id).toBe('2-2'); // A (child of B)
389387
expect(items[4].id).toBe('2-1'); // X (child of B)
390388
});
389+
390+
test('siblingCount and siblingIndex should reflect the sorted order', () => {
391+
store = Neo.create(TreeStore, {
392+
model: TestModel,
393+
data : [
394+
{ id: '1', name: 'Z', isLeaf: false, collapsed: false },
395+
{ id: '1-1', parentId: '1', name: 'B', isLeaf: true },
396+
{ id: '1-2', parentId: '1', name: 'A', isLeaf: true },
397+
{ id: '2', name: 'A', isLeaf: false, collapsed: false },
398+
{ id: '2-1', parentId: '2', name: 'Z', isLeaf: true },
399+
{ id: '2-2', parentId: '2', name: 'Y', isLeaf: true }
400+
]
401+
});
402+
403+
// Initial check
404+
let rootZ = store.get('1');
405+
expect(rootZ.siblingIndex).toBe(1);
406+
expect(rootZ.siblingCount).toBe(2);
407+
408+
let rootA = store.get('2');
409+
expect(rootA.siblingIndex).toBe(2);
410+
expect(rootA.siblingCount).toBe(2);
411+
412+
let child1_B = store.get('1-1');
413+
expect(child1_B.siblingIndex).toBe(1);
414+
expect(child1_B.siblingCount).toBe(2);
415+
416+
let child1_A = store.get('1-2');
417+
expect(child1_A.siblingIndex).toBe(2);
418+
expect(child1_A.siblingCount).toBe(2);
419+
420+
// Sort descending by name
421+
store.sorters = [{
422+
property: 'name',
423+
direction: 'DESC'
424+
}];
425+
426+
// Expected sorted roots (DESC): 'Z' (id '1'), 'A' (id '2')
427+
// Expected sorted children of 'Z' (DESC): 'B' (id '1-1'), 'A' (id '1-2')
428+
// Expected sorted children of 'A' (DESC): 'Z' (id '2-1'), 'Y' (id '2-2')
429+
430+
expect(rootZ.siblingIndex).toBe(1); // Z is still 1
431+
expect(rootA.siblingIndex).toBe(2); // A is still 2
432+
433+
expect(child1_B.siblingIndex).toBe(1);
434+
expect(child1_A.siblingIndex).toBe(2);
435+
436+
let child2_Z = store.get('2-1');
437+
let child2_Y = store.get('2-2');
438+
439+
expect(child2_Z.siblingIndex).toBe(1);
440+
expect(child2_Y.siblingIndex).toBe(2);
441+
442+
// Sort ascending by name
443+
store.sorters = [{
444+
property: 'name',
445+
direction: 'ASC'
446+
}];
447+
448+
// Expected sorted roots (ASC): 'A' (id '2'), 'Z' (id '1')
449+
// Expected sorted children of 'Z' (ASC): 'A' (id '1-2'), 'B' (id '1-1')
450+
451+
expect(rootA.siblingIndex).toBe(1);
452+
expect(rootZ.siblingIndex).toBe(2);
453+
454+
expect(child1_A.siblingIndex).toBe(1);
455+
expect(child1_B.siblingIndex).toBe(2);
456+
457+
expect(child2_Y.siblingIndex).toBe(1);
458+
expect(child2_Z.siblingIndex).toBe(2);
459+
});
391460
});
392461

393462
test.describe('Neo.data.TreeStore (Ancestor-Aware Filtering)', () => {
@@ -402,19 +471,17 @@ test.describe('Neo.data.TreeStore (Ancestor-Aware Filtering)', () => {
402471
*/
403472
let store, TestModel;
404473

405-
test.beforeEach(() => {
406-
class TestTreeModel5 extends TreeModel {
407-
static config = {
408-
className: 'Test.Unit.Data.TreeStore.TestTreeModel5',
409-
fields: [
410-
{ name: 'id', type: 'String' },
411-
{ name: 'name', type: 'String' }
412-
]
413-
}
474+
class TestTreeModel5 extends TreeModel {
475+
static config = {
476+
className: 'Test.Unit.Data.TreeStore.TestTreeModel5',
477+
fields: [
478+
{ name: 'id', type: 'String' },
479+
{ name: 'name', type: 'String' }
480+
]
414481
}
482+
}
415483

416-
TestModel = Neo.setupClass(TestTreeModel5);
417-
});
484+
TestModel = Neo.setupClass(TestTreeModel5);
418485

419486
test.afterEach(() => {
420487
store?.destroy();
@@ -484,4 +551,70 @@ test.describe('Neo.data.TreeStore (Ancestor-Aware Filtering)', () => {
484551
expect(items[0].collapsed).toBe(false);
485552
expect(items[1].collapsed).toBe(false);
486553
});
554+
555+
test('siblingCount and siblingIndex should reflect only visible siblings when filtered', () => {
556+
store = Neo.create(TreeStore, {
557+
model: TestModel,
558+
data : [
559+
{ id: '1', name: 'Root A', isLeaf: false, collapsed: true },
560+
{ id: '1-1', parentId: '1', name: 'Child A1', isLeaf: false, collapsed: true },
561+
{ id: '1-1-1', parentId: '1-1', name: 'Target Node', isLeaf: true },
562+
{ id: '1-1-2', parentId: '1-1', name: 'Other Node', isLeaf: true },
563+
{ id: '2', name: 'Root B', isLeaf: false, collapsed: true },
564+
{ id: '2-1', parentId: '2', name: 'Child B1', isLeaf: true }
565+
]
566+
});
567+
568+
// Check pre-filter stats
569+
expect(store.get('1-1-1').siblingCount).toBe(2);
570+
expect(store.get('1-1-2').siblingCount).toBe(2);
571+
572+
expect(store.get('1').siblingCount).toBe(2);
573+
expect(store.get('2').siblingCount).toBe(2);
574+
575+
// Filter for 'Target Node'
576+
store.filters = [{
577+
property: 'name',
578+
value : 'Target Node'
579+
}];
580+
581+
// Only Root A -> Child A1 -> Target Node are visible
582+
583+
let rootA = store.get('1');
584+
let childA1 = store.get('1-1');
585+
let targetNode = store.get('1-1-1');
586+
587+
// Root A is now the only visible root sibling (Root B is hidden)
588+
expect(rootA.siblingCount).toBe(1);
589+
expect(rootA.siblingIndex).toBe(1);
590+
591+
// Child A1 is the only visible child of Root A
592+
expect(childA1.siblingCount).toBe(1);
593+
expect(childA1.siblingIndex).toBe(1);
594+
595+
// Target Node is the only visible child of Child A1
596+
expect(targetNode.siblingCount).toBe(1);
597+
expect(targetNode.siblingIndex).toBe(1);
598+
599+
// Verify childCount on parents is correctly updated to reflect only visible children
600+
expect(rootA.childCount).toBe(1); // Only Child A1 is visible
601+
expect(childA1.childCount).toBe(1); // Only Target Node is visible
602+
603+
// Let's clear the filter and ensure stats are restored
604+
store.filters = [];
605+
606+
expect(store.get('1-1-1').siblingCount).toBe(2);
607+
expect(store.get('1-1-1').siblingIndex).toBe(1);
608+
expect(store.get('1-1-2').siblingCount).toBe(2);
609+
expect(store.get('1-1-2').siblingIndex).toBe(2);
610+
611+
expect(store.get('1').siblingCount).toBe(2);
612+
expect(store.get('1').siblingIndex).toBe(1);
613+
expect(store.get('2').siblingCount).toBe(2);
614+
expect(store.get('2').siblingIndex).toBe(2);
615+
616+
// Verify childCount is restored to structural counts
617+
expect(store.get('1').childCount).toBe(1); // Child A1
618+
expect(store.get('1-1').childCount).toBe(2); // Target Node and Other Node
619+
});
487620
});

0 commit comments

Comments
 (0)