From e165fa3a870db121f4f55ad85d34aed9b30185cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Malinowski?= Date: Tue, 26 Nov 2024 17:18:16 +0100 Subject: [PATCH 1/5] test(multi-select): add test suite --- tests/MultiSelect/MultiSelect.test.ts | 124 ++++++++++++++++++++++++++ tests/setup-tests.ts | 3 + 2 files changed, 127 insertions(+) create mode 100644 tests/MultiSelect/MultiSelect.test.ts diff --git a/tests/MultiSelect/MultiSelect.test.ts b/tests/MultiSelect/MultiSelect.test.ts new file mode 100644 index 0000000000..aed26b4a52 --- /dev/null +++ b/tests/MultiSelect/MultiSelect.test.ts @@ -0,0 +1,124 @@ +import { render, screen } from "@testing-library/svelte"; +import { MultiSelect } from "carbon-components-svelte"; +import { user } from "../setup-tests"; + +describe("MultiSelect sorts items correctly", () => { + /** Opens the dropdown. */ + const openMenu = async () => + await user.click( + await screen.findByLabelText("Open menu", { + selector: `[role="button"]`, + }), + ); + + /** Closes the dropdown. */ + const closeMenu = async () => + await user.click( + await screen.findByLabelText("Close menu", { + selector: `[role="button"]`, + }), + ); + + /** Toggles an option, identifying it by its `text` value. */ + const toggleOption = async (optionText: string) => + await user.click( + await screen.findByText((text) => text.trim() === optionText), + ); + + /** Fetches the `text` value of the nth option in the MultiSelect component. */ + const nthRenderedOptionText = (index: number) => + screen.queryAllByRole("option").at(index)?.textContent?.trim(); + + it("initially sorts items alphabetically", async () => { + render(MultiSelect, { + items: [ + { id: "1", text: "C" }, + { id: "3", text: "A" }, + { id: "2", text: "B" }, + ], + }); + + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + expect(nthRenderedOptionText(1)).toBe("B"); + expect(nthRenderedOptionText(2)).toBe("C"); + }); + + it("immediately moves selected items to the top (with selectionFeedback: top)", async () => { + render(MultiSelect, { + items: [ + { id: "3", text: "C" }, + { id: "1", text: "A" }, + { id: "2", text: "B" }, + ], + selectionFeedback: "top", + }); + + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + + await toggleOption("C"); + expect(nthRenderedOptionText(0)).toBe("C"); + + await toggleOption("C"); + expect(nthRenderedOptionText(0)).toBe("A"); + }); + + it("sorts newly-toggled items only after the dropdown is reoponed (with selectionFeedback: top-after-reopen)", async () => { + render(MultiSelect, { + items: [ + { id: "3", text: "C" }, + { id: "1", text: "A" }, + { id: "2", text: "B" }, + ], + }); + + // Initially, items should be sorted alphabetically. + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + + // While the menu is still open, a newly-selected item should not move. + await toggleOption("C"); + expect(nthRenderedOptionText(0)).toBe("A"); + + // The newly-selected item should move after the menu is closed and + // re-opened. + await closeMenu(); + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("C"); + + // A deselected item should not move while the dropdown is still open. + await toggleOption("C"); + expect(nthRenderedOptionText(0)).toBe("C"); + + // The deselected item should move after closing and re-opening the dropdown. + await closeMenu(); + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + }); + + it("never moves selected items to the top (with selectionFeedback: fixed)", async () => { + render(MultiSelect, { + items: [ + { id: "3", text: "C" }, + { id: "1", text: "A" }, + { id: "2", text: "B" }, + ], + selectionFeedback: "fixed", + }); + + // Items should be sorted alphabetically. + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + + // A newly-selected item should not move after the selection is made. + await toggleOption("C"); + expect(nthRenderedOptionText(0)).toBe("A"); + + // The newly-selected item also shouldn’t move after the dropdown is closed + // and reopened. + await closeMenu(); + await openMenu(); + expect(nthRenderedOptionText(0)).toBe("A"); + }); +}); diff --git a/tests/setup-tests.ts b/tests/setup-tests.ts index d494d2af52..c69af71048 100644 --- a/tests/setup-tests.ts +++ b/tests/setup-tests.ts @@ -3,4 +3,7 @@ import "@testing-library/jest-dom/vitest"; import { userEvent } from "@testing-library/user-event"; import "../css/all.css"; +// Mock scrollIntoView since it's not implemented in JSDOM +Element.prototype.scrollIntoView = vi.fn(); + export const user = userEvent.setup(); From c3a390f3fef072c6b736e33a85a2ae772df12e52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Malinowski?= Date: Thu, 28 Nov 2024 20:50:15 +0100 Subject: [PATCH 2/5] fix(multi-select): fix sorting behavior - Menu items are sorted when the component renders. - With selectionFeedback: top, selected items are immediately pinned to the top. - With selectionFeedback: top-after-reopen, selected items are pinned to the top only after the dropdown is closed. - With selectionFeedback: fixed, selected items are never pinned to the top. Fixes #2066 --- src/MultiSelect/MultiSelect.svelte | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/MultiSelect/MultiSelect.svelte b/src/MultiSelect/MultiSelect.svelte index abbdf7721f..8fa8abed54 100644 --- a/src/MultiSelect/MultiSelect.svelte +++ b/src/MultiSelect/MultiSelect.svelte @@ -243,11 +243,11 @@ } }); - $: menuId = `menu-${id}`; - $: inline = type === "inline"; - $: ariaLabel = $$props["aria-label"] || "Choose an item"; - $: sortedItems = (() => { - if (selectionFeedback === "top" && selectedIds.length > 0) { + function sort() { + if ( + selectionFeedback === "top" || + selectionFeedback === "top-after-reopen" + ) { const checkedItems = items .filter((item) => selectedIds.includes(item.id)) .map((item) => ({ ...item, checked: true })); @@ -269,7 +269,20 @@ checked: selectedIds.includes(item.id), })) .sort(sortItem); - })(); + } + + let sortedItems = sort(); + + $: menuId = `menu-${id}`; + $: inline = type === "inline"; + $: ariaLabel = $$props["aria-label"] || "Choose an item"; + $: if ( + selectedIds && + (selectionFeedback === "top" || + (selectionFeedback === "top-after-reopen" && open === false)) + ) { + sortedItems = sort(); + } $: checked = sortedItems.filter(({ checked }) => checked); $: unchecked = sortedItems.filter(({ checked }) => !checked); $: filteredItems = sortedItems.filter((item) => filterItem(item, value)); From 765ffc88eb39a02dbe0b78cc9e4f1cc7b0c4077c Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Sat, 30 Nov 2024 10:39:34 -0800 Subject: [PATCH 3/5] chore: run lint --- docs/src/COMPONENT_API.json | 7 ++---- docs/src/components/ComponentApi.svelte | 30 +++++++++++++++---------- scripts/format-component-api.js | 6 ++--- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/docs/src/COMPONENT_API.json b/docs/src/COMPONENT_API.json index c6ceb31d19..469dea2d09 100644 --- a/docs/src/COMPONENT_API.json +++ b/docs/src/COMPONENT_API.json @@ -3235,10 +3235,7 @@ "ts": "interface DataTableCell<\n Row = DataTableRow,\n> {\n key:\n | DataTableKey\n | (string & {});\n value: DataTableValue;\n display?: (\n item: DataTableValue,\n row: DataTableRow,\n ) => DataTableValue;\n}\n" } ], - "generics": [ - "Row", - "Row extends DataTableRow = DataTableRow" - ], + "generics": ["Row", "Row extends DataTableRow = DataTableRow"], "rest_props": { "type": "Element", "name": "div" @@ -18066,4 +18063,4 @@ } } ] -} \ No newline at end of file +} diff --git a/docs/src/components/ComponentApi.svelte b/docs/src/components/ComponentApi.svelte index 54dd1cd301..d819418677 100644 --- a/docs/src/components/ComponentApi.svelte +++ b/docs/src/components/ComponentApi.svelte @@ -39,10 +39,10 @@ $: source = `https://github.com/carbon-design-system/carbon-components-svelte/tree/master/${component.filePath}`; $: forwarded_events = component.events.filter( - (event) => event.type === "forwarded" + (event) => event.type === "forwarded", ); $: dispatched_events = component.events.filter( - (event) => event.type === "dispatched" + (event) => event.type === "dispatched", ); @@ -116,20 +116,20 @@ > - + type="inline" + code={typeMap[type]} + /> + {:else}
-
+ type="inline" + code={type} + /> + {/if} {/each} @@ -145,10 +145,16 @@ {/each} {/if} -
+
Default value
-
+
{#if prop.value === undefined} undefined {:else} diff --git a/scripts/format-component-api.js b/scripts/format-component-api.js index 537d3a3b77..ddf8bdafcf 100644 --- a/scripts/format-component-api.js +++ b/scripts/format-component-api.js @@ -5,11 +5,11 @@ import { format } from "prettier"; import plugin from "prettier/plugins/typescript"; const formatTypeScript = async (value) => { - return await format(value, { - parser: "typescript", + return await format(value, { + parser: "typescript", plugins: [plugin], printWidth: 40, // Force breaking onto new lines - bracketSameLine: false + bracketSameLine: false, }); }; From 09541657ef530be6616683f5b0f9b6916ea60e6c Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Sat, 30 Nov 2024 10:40:37 -0800 Subject: [PATCH 4/5] ci: run `lint` check --- .github/workflows/checks.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 9f9b6c62e0..df91e1b935 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -17,6 +17,9 @@ jobs: - name: Install dependencies run: npm install + - name: Run lint + run: npm run lint + - name: Unit tests run: npm run test From c7f4b16bf36a71e5d0add4289b567b54ee3f5add Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Sat, 30 Nov 2024 10:48:32 -0800 Subject: [PATCH 5/5] v0.86.2 --- CHANGELOG.md | 6 ++++++ COMPONENT_INDEX.md | 2 +- package-lock.json | 4 ++-- package.json | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32f09f9a7f..796d8ced13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +### [0.86.2](https://github.com/carbon-design-system/carbon-components-svelte/compare/v0.86.1...v0.86.2) (2024-11-30) + +### Bug Fixes + +- **multi-select:** fix sorting behavior ([c3a390f](https://github.com/carbon-design-system/carbon-components-svelte/commit/c3a390f3fef072c6b736e33a85a2ae772df12e52)), closes [#2066](https://github.com/carbon-design-system/carbon-components-svelte/issues/2066) + ## [0.86.1](https://github.com/carbon-design-system/carbon-components-svelte/compare/v0.86.0...v0.86.1) (2024-11-22) ### Bug Fixes diff --git a/COMPONENT_INDEX.md b/COMPONENT_INDEX.md index c7c72fca87..3cd4849125 100644 --- a/COMPONENT_INDEX.md +++ b/COMPONENT_INDEX.md @@ -1,6 +1,6 @@ # Component Index -> 165 components exported from carbon-components-svelte@0.86.1. +> 165 components exported from carbon-components-svelte@0.86.2. ## Components diff --git a/package-lock.json b/package-lock.json index d2643eb39d..be0bba0882 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "carbon-components-svelte", - "version": "0.86.1", + "version": "0.86.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "carbon-components-svelte", - "version": "0.86.1", + "version": "0.86.2", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index 32af01ac47..c779262f31 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "carbon-components-svelte", - "version": "0.86.1", + "version": "0.86.2", "license": "Apache-2.0", "description": "Svelte implementation of the Carbon Design System", "type": "module",