Merge branch 'next' into select

docs/resources/changelog.md

@@ -12,8 +12,9 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 
 This release includes a complete rewrite of `<sl-select>` to improve accessibility and improve simplify the internal structure.
 
-- 🚨 BREAKING: removed the `multiple` attribute from `<sl-select>` because it was inaccessible and made the getting/setting the value inconsistent and confusing (see the docs for a suggested multiselect pattern)
 - 🚨 BREAKING: removed the `suffix` slot from `<sl-select>` because it was confusing to users and its position made the clear button inaccessible
+- 🚨 BREAKING: removed the `multiple` attribute from `<sl-select>` because it was inaccessible and made the getting/setting the value inconsistent and confusing (see the docs for a suggested multiselect pattern)
+- Fixed a bug in `<sl-tree-item>` where the checked/indeterminate states could get out of sync when using the `multiple` option [#1076](https://github.com/shoelace-style/shoelace/issues/1076)
 
 ## 2.0.0-beta.87
 

src/components/tree/tree.test.ts

@@ -655,4 +655,98 @@ describe('<sl-tree>', () => {
       });
     });
   });
+
+  describe('Checkboxes synchronization', () => {
+    describe('when the tree gets initialized', () => {
+      describe('and a parent node is selected', () => {
+        it('should select all the nested children', async () => {
+          // Arrange
+          const tree = await fixture<SlTree>(html`
+            <sl-tree selection="multiple">
+              <sl-tree-item selected>
+                Parent Node
+                <sl-tree-item selected>Child Node 1</sl-tree-item>
+                <sl-tree-item>
+                  Child Node 2
+                  <sl-tree-item>Child Node 2 - 1</sl-tree-item>
+                  <sl-tree-item>Child Node 2 - 2</sl-tree-item>
+                </sl-tree-item>
+              </sl-tree-item>
+            </sl-tree>
+          `);
+          const treeItems = Array.from<SlTreeItem>(tree.querySelectorAll('sl-tree-item'));
+
+          // Act
+          await tree.updateComplete;
+
+          // Assert
+          treeItems.forEach(treeItem => {
+            expect(treeItem).to.have.attribute('selected');
+          });
+        });
+      });
+
+      describe('and a parent node is not selected', () => {
+        describe('and all the children are selected', () => {
+          it('should select the parent node', async () => {
+            // Arrange
+            const tree = await fixture<SlTree>(html`
+              <sl-tree selection="multiple">
+                <sl-tree-item>
+                  Parent Node
+                  <sl-tree-item selected>Child Node 1</sl-tree-item>
+                  <sl-tree-item selected>
+                    Child Node 2
+                    <sl-tree-item>Child Node 2 - 1</sl-tree-item>
+                    <sl-tree-item>Child Node 2 - 2</sl-tree-item>
+                  </sl-tree-item>
+                </sl-tree-item>
+              </sl-tree>
+            `);
+            const treeItems = Array.from<SlTreeItem>(tree.querySelectorAll('sl-tree-item'));
+
+            // Act
+            await tree.updateComplete;
+
+            // Assert
+            treeItems.forEach(treeItem => {
+              expect(treeItem).to.have.attribute('selected');
+            });
+            expect(treeItems[0].indeterminate).to.be.false;
+          });
+        });
+
+        describe('and some of the children are selected', () => {
+          it('should set the parent node to indeterminate state', async () => {
+            // Arrange
+            const tree = await fixture<SlTree>(html`
+              <sl-tree selection="multiple">
+                <sl-tree-item>
+                  Parent Node
+                  <sl-tree-item selected>Child Node 1</sl-tree-item>
+                  <sl-tree-item>
+                    Child Node 2
+                    <sl-tree-item>Child Node 2 - 1</sl-tree-item>
+                    <sl-tree-item>Child Node 2 - 2</sl-tree-item>
+                  </sl-tree-item>
+                </sl-tree-item>
+              </sl-tree>
+            `);
+            const treeItems = Array.from<SlTreeItem>(tree.querySelectorAll('sl-tree-item'));
+
+            // Act
+            await tree.updateComplete;
+
+            // Assert
+            expect(treeItems[0]).not.to.have.attribute('selected');
+            expect(treeItems[0].indeterminate).to.be.true;
+            expect(treeItems[1]).to.have.attribute('selected');
+            expect(treeItems[2]).not.to.have.attribute('selected');
+            expect(treeItems[3]).not.to.have.attribute('selected');
+            expect(treeItems[4]).not.to.have.attribute('selected');
+          });
+        });
+      });
+    });
+  });
 });

src/components/tree/tree.ts

@@ -8,31 +8,44 @@ import SlTreeItem from '../tree-item/tree-item';
 import styles from './tree.styles';
 import type { CSSResultGroup } from 'lit';
 
-function syncCheckboxes(changedTreeItem: SlTreeItem) {
+function syncCheckboxes(changedTreeItem: SlTreeItem, initialSync = false) {
+  function syncParentItem(treeItem: SlTreeItem) {
+    const children = treeItem.getChildrenItems({ includeDisabled: false });
+
+    if (children.length) {
+      const allChecked = children.every(item => item.selected);
+      const allUnchecked = children.every(item => !item.selected && !item.indeterminate);
+
+      treeItem.selected = allChecked;
+      treeItem.indeterminate = !allChecked && !allUnchecked;
+    }
+  }
+
   function syncAncestors(treeItem: SlTreeItem) {
     const parentItem: SlTreeItem | null = treeItem.parentElement as SlTreeItem;
 
     if (SlTreeItem.isTreeItem(parentItem)) {
-      const children = parentItem.getChildrenItems({ includeDisabled: false });
-      const allChecked = !!children.length && children.every(item => item.selected);
-      const allUnchecked = children.every(item => !item.selected && !item.indeterminate);
-
-      parentItem.selected = allChecked;
-      parentItem.indeterminate = !allChecked && !allUnchecked;
-
+      syncParentItem(parentItem);
       syncAncestors(parentItem);
     }
   }
 
   function syncDescendants(treeItem: SlTreeItem) {
     for (const childItem of treeItem.getChildrenItems()) {
-      childItem.selected = !childItem.disabled && treeItem.selected;
+      childItem.selected = initialSync
+        ? treeItem.selected || childItem.selected
+        : !childItem.disabled && treeItem.selected;
+
       syncDescendants(childItem);
     }
+
+    if (initialSync) {
+      syncParentItem(treeItem);
+    }
   }
 
-  syncAncestors(changedTreeItem);
   syncDescendants(changedTreeItem);
+  syncAncestors(changedTreeItem);
 }
 
 /**
@@ -88,6 +101,7 @@ export default class SlTree extends ShoelaceElement {
     this.addEventListener('sl-lazy-change', this.handleSlotChange);
 
     await this.updateComplete;
+
     this.mutationObserver = new MutationObserver(this.handleTreeChanged);
     this.mutationObserver.observe(this, { childList: true, subtree: true });
   }
@@ -155,13 +169,22 @@ export default class SlTree extends ShoelaceElement {
   };
 
   @watch('selection')
-  handleSelectionChange() {
+  async handleSelectionChange() {
+    const isSelectionMultiple = this.selection === 'multiple';
     const items = this.getAllTreeItems();
 
-    this.setAttribute('aria-multiselectable', this.selection === 'multiple' ? 'true' : 'false');
+    this.setAttribute('aria-multiselectable', isSelectionMultiple ? 'true' : 'false');
 
     for (const item of items) {
-      item.selectable = this.selection === 'multiple';
+      item.selectable = isSelectionMultiple;
+    }
+
+    if (isSelectionMultiple) {
+      await this.updateComplete;
+
+      [...this.querySelectorAll(':scope > sl-tree-item')].forEach((treeItem: SlTreeItem) =>
+        syncCheckboxes(treeItem, true)
+      );
     }
   }