Refactor tree

.changeset/better-planes-stick.md

@@ -0,0 +1,7 @@
+---
+'@sl-design-system/tree': minor
+---
+
+Replace `@lit-labs/virtualizer` with `@tanstack/lit-virtual`
+
+This is a breaking change, but not worthy of a new major version, because the component still has draft status.

.changeset/fancy-llamas-repeat.md

@@ -0,0 +1,8 @@
+---
+'@sl-design-system/button-bar': minor
+---
+`
+Add `fill` and `variant` properties, similar to `size` that will be applied
+to all child buttons. This can especially be useful when embedding the button bar
+in another component where the buttons are slotted. This way the developer doesn't
+have to set the same properties on each button manually.

.changeset/tangy-mammals-shave.md

@@ -0,0 +1,5 @@
+---
+'@sl-design-system/shared': patch
+---
+
+Fix `scrollParent` to work with overflow elements that do not scroll (yet)

.github/copilot-instructions.md

@@ -45,7 +45,7 @@ yarn build
 
 To test a specific component, use the following command from the root of the repository:
 ```bash
-wtr packages/components/<component>/src/<component>.spec.ts
+vitest packages/components/<component>/src/<component>.spec.ts
 ```
 
 To run linting and formatting checks, use the following command from the root of the repository:

.yarn/patches/@tanstack-lit-virtual-npm-3.13.12-56dd05e0b4.patch

@@ -0,0 +1,62 @@
+diff --git a/dist/cjs/index.cjs b/dist/cjs/index.cjs
+index 01a42de29d1f945abbefa4199f1337f9640a9020..de33416b0a327099bf0931358dcad4b131e616ce 100644
+--- a/dist/cjs/index.cjs
++++ b/dist/cjs/index.cjs
+@@ -5,7 +5,6 @@ class VirtualizerControllerBase {
+   constructor(host, options) {
+     this.cleanup = () => {
+     };
+-    (this.host = host).addController(this);
+     const resolvedOptions = {
+       ...options,
+       onChange: (instance, sync) => {
+@@ -15,6 +14,7 @@ class VirtualizerControllerBase {
+       }
+     };
+     this.virtualizer = new virtualCore.Virtualizer(resolvedOptions);
++    (this.host = host).addController(this);
+   }
+   getVirtualizer() {
+     return this.virtualizer;
+diff --git a/dist/esm/index.js b/dist/esm/index.js
+index 385e7a480460f0468447e2cd8553baca48238c7f..e2baae311feadb842c2c826d4e805d7251867120 100644
+--- a/dist/esm/index.js
++++ b/dist/esm/index.js
+@@ -3,7 +3,6 @@ class VirtualizerControllerBase {
+   constructor(host, options) {
+     this.cleanup = () => {
+     };
+-    (this.host = host).addController(this);
+     const resolvedOptions = {
+       ...options,
+       onChange: (instance, sync) => {
+@@ -13,6 +12,7 @@ class VirtualizerControllerBase {
+       }
+     };
+     this.virtualizer = new Virtualizer(resolvedOptions);
++    (this.host = host).addController(this);
+   }
+   getVirtualizer() {
+     return this.virtualizer;
+diff --git a/src/index.ts b/src/index.ts
+index 7a00af057f1b1da31ebfedd7dfd4544d99791ead..e36887fd05ca042241f474479b5e5754626fa8b8 100644
+--- a/src/index.ts
++++ b/src/index.ts
+@@ -25,8 +25,6 @@ class VirtualizerControllerBase<
+     host: ReactiveControllerHost,
+     options: VirtualizerOptions<TScrollElement, TItemElement>,
+   ) {
+-    ;(this.host = host).addController(this)
+-
+     const resolvedOptions: VirtualizerOptions<TScrollElement, TItemElement> = {
+       ...options,
+       onChange: (instance, sync) => {
+@@ -35,6 +33,8 @@ class VirtualizerControllerBase<
+       },
+     }
+     this.virtualizer = new Virtualizer(resolvedOptions)
++
++    ;(this.host = host).addController(this)
+   }
+
+   public getVirtualizer() {

packages/angular/stories/wrappers.stories.ts

@@ -348,7 +348,7 @@ export const Tree: StoryObj = {
       getLevel: ({ level }: Item) => level,
       isExpandable: ({ expandable }: Item) => expandable,
       isExpanded: ({ name }: Item) => ['tree', 'src'].includes(name),
-      selects: 'multiple'
+      multiple: true
     });
 
     return {

packages/components/button-bar/src/button-bar.spec.ts

@@ -63,6 +63,24 @@ describe('sl-button-bar', () => {
 
       expect(buttons.map(b => b.size)).to.deep.equal(['lg', 'lg', 'lg']);
     });
+
+    it('should propagate fill to the buttons', async () => {
+      const buttons = Array.from(el.querySelectorAll('sl-button'));
+
+      el.fill = 'ghost';
+      await el.updateComplete;
+
+      expect(buttons.map(b => b.fill)).to.deep.equal(['ghost', 'ghost', 'ghost']);
+    });
+
+    it('should propagate variant to the buttons', async () => {
+      const buttons = Array.from(el.querySelectorAll('sl-button'));
+
+      el.variant = 'primary';
+      await el.updateComplete;
+
+      expect(buttons.map(b => b.variant)).to.deep.equal(['primary', 'primary', 'primary']);
+    });
   });
 
   describe('icon only', () => {

packages/components/button-bar/src/button-bar.ts

@@ -1,4 +1,4 @@
-import { type ButtonSize } from '@sl-design-system/button';
+import { type ButtonFill, type ButtonSize, type ButtonVariant } from '@sl-design-system/button';
 import { type CSSResultGroup, LitElement, type PropertyValues, ReactiveElement, type TemplateResult, html } from 'lit';
 import { property } from 'lit/decorators.js';
 import styles from './button-bar.scss.js';
@@ -29,28 +29,46 @@ export class ButtonBar extends LitElement {
 
   /**
    * The alignment of the buttons within the bar.
-   * @default start
+   * @default 'start'
    */
   @property({ reflect: true }) align?: ButtonBarAlign;
 
+  /**
+   * Determines the fill of all buttons in the bar.
+   * @default undefined
+   */
+  @property() fill?: ButtonFill;
+
   /**
    * Whether the bar only contains icon-only buttons.
    * Determined based on the actual content, so does not need to be set.
    * @internal
    */
   @property({ type: Boolean, reflect: true, attribute: 'icon-only' }) iconOnly?: boolean;
 
-  /** When set to true, the button order is reversed. */
+  /**
+   * When set to true, the button order is reversed.
+   * @default false
+   */
   @property({ type: Boolean, reflect: true }) reverse?: boolean;
 
-  /** Determines the size of all buttons in the bar. */
+  /**
+   * Determines the size of all buttons in the bar.
+   * @default undefined
+   */
   @property() size?: ButtonSize;
 
+  /**
+   * Determines the variant of all buttons in the bar.
+   * @default undefined
+   */
+  @property() variant?: ButtonVariant;
+
   override updated(changes: PropertyValues<this>): void {
     super.updated(changes);
 
-    if (changes.has('size')) {
-      this.#updateButtonSize();
+    if (changes.has('fill') || changes.has('size') || changes.has('variant')) {
+      this.#updateButtons();
     }
   }
 
@@ -73,19 +91,27 @@ export class ButtonBar extends LitElement {
 
     this.iconOnly = icons.every(Boolean);
 
-    this.#updateButtonSize();
+    this.#updateButtons();
   }
 
-  #updateButtonSize(): void {
+  #updateButtons(): void {
     this.renderRoot
       .querySelector('slot')
       ?.assignedElements({ flatten: true })
       .forEach(element => {
-        const button = element as { size?: ButtonSize };
+        const button = element as { fill?: ButtonFill; size?: ButtonSize; variant?: ButtonVariant };
 
         if (this.size) {
           button.size = this.size;
         }
+
+        if (this.fill) {
+          button.fill = this.fill;
+        }
+
+        if (this.variant) {
+          button.variant = this.variant;
+        }
       });
   }
 }

packages/components/data-source/src/data-source.ts

@@ -10,7 +10,7 @@ export type DataSourceFilterFunction<Model> = (item: Model, value: unknown) => b
 
 export type DataSourceFilter<Model> = {
   id: string;
-  by: DataSourceFilterFunction<Model> | PathKeys<Model>;
+  by: DataSourceFilterFunction<Model> | PathKeys<Model> | string;
   value?: unknown;
 };
 
@@ -20,7 +20,7 @@ export type DataSourceSortDirection = 'asc' | 'desc';
 export type DataSourceSortFunction<Model = any> = (a: Model, b: Model) => number;
 
 export type DataSourceSort<Model> = {
-  by: DataSourceSortFunction<Model> | PathKeys<Model>;
+  by: DataSourceSortFunction<Model> | PathKeys<Model> | string;
   direction: DataSourceSortDirection;
 };
 

packages/components/shared/src/dom.ts

@@ -4,7 +4,10 @@
  * @returns The first scrollable parent of the given element; if no explicit scroll parent, returns the html element.
  */
 export const getScrollParent = (element: Element): Element => {
-  if (element.scrollHeight > element.clientHeight) {
+  const { overflow, overflowY } = getComputedStyle(element),
+    scrollable = /(auto|scroll)/.test(overflow + overflowY);
+
+  if (scrollable || element.scrollHeight > element.clientHeight) {
     return element;
   } else if (element.parentElement) {
     return getScrollParent(element.parentElement);

packages/components/tree/package.json

@@ -48,10 +48,12 @@
   },
   "devDependencies": {
     "@open-wc/scoped-elements": "^3.0.6",
+    "@tanstack/lit-virtual": "patch:@tanstack/lit-virtual@npm%3A3.13.12#~/.yarn/patches/@tanstack-lit-virtual-npm-3.13.12-56dd05e0b4.patch",
     "lit": "^3.3.1"
   },
   "peerDependencies": {
     "@open-wc/scoped-elements": "^3.0.6",
+    "@tanstack/lit-virtual": "^3.13.12",
     "lit": "^3.1.4"
   }
 }

packages/components/tree/src/flat-tree-data-source.spec.ts

@@ -23,12 +23,65 @@ describe('FlatTreeDataSource', () => {
       );
     });
 
-    it('should not be selectable', () => {
-      expect(ds.selects).to.be.undefined;
-    });
-
     it('should have the correct size', () => {
       expect(ds.size).to.equal(2);
     });
   });
+
+  describe('level guides', () => {
+    beforeEach(() => {
+      /**
+       * Tree structure (do not show the root; use ASCII art for the indent guides):
+       * A (level 0, not last)
+       * ├─ A.1 (level 1, not last)
+       * └─ A.2 (level 1, last child)
+       *    └─ A.2.a (level 2, last child)
+       *       └─ A.2.a.1 (level 3, last child)
+       * B (level 0, last child)
+       */
+      ds = new FlatTreeDataSource(
+        [
+          { id: 'A', name: 'A', level: 0, expandable: true },
+          { id: 'A.1', name: 'A.1', level: 1, expandable: false },
+          { id: 'A.2', name: 'A.2', level: 1, expandable: true },
+          { id: 'A.2.a', name: 'A.2.a', level: 2, expandable: true },
+          { id: 'A.2.a.1', name: 'A.2.a.1', level: 3, expandable: false },
+          { id: 'B', name: 'B', level: 0, expandable: false }
+        ],
+        {
+          getId: ({ id }) => id,
+          getLabel: ({ name }) => name,
+          getLevel: ({ level }) => level,
+          isExpandable: ({ expandable }) => expandable,
+          isExpanded: () => true
+        }
+      );
+      ds.update();
+    });
+
+    it('should set the correct levels', () => {
+      const levels = ds.items.map(n => n.level);
+
+      expect(levels).to.deep.equal([0, 1, 1, 2, 3, 0]);
+    });
+
+    it('should set the correct lastNodeInLevel values', () => {
+      const lastNodeInLevels = ds.items.map(n => n.lastNodeInLevel);
+
+      expect(lastNodeInLevels).to.deep.equal([false, false, true, true, true, true]);
+    });
+
+    it('should set the correct levelGuides values', () => {
+      const levelGuides = ds.items.map(n => n.levelGuides);
+
+      expect(levelGuides).to.deep.equal([
+        [], // A
+        [0], // A.1
+        [0], // A.2
+        [1], // A.2.a
+        [2], // A.2.a.1
+        [] // B
+      ]);
+    });
+  });
 });

packages/components/tree/src/flat-tree-data-source.ts

@@ -10,8 +10,11 @@ export interface FlatTreeDataSourceMapping<T> extends TreeDataSourceMapping<T> {
 }
 
 export interface FlatTreeDataSourceOptions<T> extends FlatTreeDataSourceMapping<T> {
+  /** Provide this method to lazy load child nodes when a parent node is expanded. */
   loadChildren?(node: T): Promise<T[]>;
-  selects?: 'single' | 'multiple';
+
+  /** Enables multiple selection of tree nodes. */
+  multiple?: boolean;
 }
 
 /**
@@ -53,6 +56,7 @@ export class FlatTreeDataSource<T = any> extends TreeDataSource<T> {
     super({ ...options, loadChildren });
 
     this.#mapping = {
+      getAriaDescription: options.getAriaDescription,
       getChildrenCount: options.getChildrenCount,
       getIcon: options.getIcon,
       getId: options.getId ?? (item => item),
@@ -65,7 +69,7 @@ export class FlatTreeDataSource<T = any> extends TreeDataSource<T> {
 
     this.#nodes = this.#mapToTreeNodes(items);
 
-    if (this.selects === 'multiple') {
+    if (this.multiple) {
       Array.from(this.selection)
         .filter(node => node.parent)
         .forEach(node => {
@@ -119,13 +123,23 @@ export class FlatTreeDataSource<T = any> extends TreeDataSource<T> {
   }
 
   #mapToTreeNode(item: T, parent?: TreeDataSourceNode<T>, lastNodeInLevel?: boolean): TreeDataSourceNode<T> {
-    const { getChildrenCount, getIcon, getId, getLabel, getLevel, isExpandable, isExpanded, isSelected } =
-      this.#mapping;
+    const {
+      getAriaDescription,
+      getChildrenCount,
+      getIcon,
+      getId,
+      getLabel,
+      getLevel,
+      isExpandable,
+      isExpanded,
+      isSelected
+    } = this.#mapping;
 
     const treeNode: TreeDataSourceNode<T> = {
       id: getId(item),
       childrenCount: getChildrenCount?.(item),
       dataNode: item,
+      description: getAriaDescription?.(item),
       expandable: isExpandable(item),
       expanded: isExpanded?.(item) ?? false,
       expandedIcon: getIcon?.(item, true),