From 88619e3da6f95f191bcd174955155865a7de17b2 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Tue, 21 Nov 2023 16:33:23 +0100 Subject: [PATCH 1/8] fix: Prevent table select event from being emitted in some cases #2196 --- ui/src/table.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ui/src/table.tsx b/ui/src/table.tsx index e47105ab78..28e54166b3 100644 --- a/ui/src/table.tsx +++ b/ui/src/table.tsx @@ -578,6 +578,8 @@ const onRenderItemColumn={onRenderItemColumn} onRenderDetailsHeader={onRenderDetailsHeader} checkboxVisibility={checkboxVisibilityMap[m.checkbox_visibility || 'on-hover']} + // Prevent selection from being cleared when reference to 'items' changes. + setKey='wave-table-items' /> {colContextMenuList && } @@ -968,18 +970,22 @@ export const React.useEffect(() => { wave.args[m.name] = [] + skipNextEventEmit.current = true + // HACK: Fluent.DetailsList fires 'onSelectionChanged' event when its internal 'isAllSelected' state changes. + // When we set 'filteredItems' to [] (e.g. by searching for a string that doesn't exist in the table), + // the 'isAllSelected' changes from 'undefined' to 'false'. This fires 'onSelectionChanged' event which emits wave 'select' event. + // We don't want 'select' event to be fired in this case, so we manually set 'isAllSelected' to 'false' on the first render + // while skipping the wave 'select' event emit by setting 'skipNextEventEmit.current' to true. + selection.setAllSelected(false) if (isSingle && m.value) { - skipNextEventEmit.current = true selection.setKeySelected(m.value, true, false) - skipNextEventEmit.current = false wave.args[m.name] = [m.value] } else if (isMultiple && m.values) { - skipNextEventEmit.current = true m.values.forEach(v => selection.setKeySelected(v, true, false)) - skipNextEventEmit.current = false wave.args[m.name] = m.values } + skipNextEventEmit.current = false // eslint-disable-next-line react-hooks/exhaustive-deps }, []) From 0347d5416d641eefb3c48aa2391dfb5fe58d8a67 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Tue, 21 Nov 2023 17:05:31 +0100 Subject: [PATCH 2/8] chore: update tests #2196 --- ui/src/table.test.tsx | 70 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/ui/src/table.test.tsx b/ui/src/table.test.tsx index 6ffd3df68a..a614c983e3 100644 --- a/ui/src/table.test.tsx +++ b/ui/src/table.test.tsx @@ -360,6 +360,76 @@ describe('Table.tsx', () => { expect(emitMock).toHaveBeenCalledTimes(1) }) + it('Does not fire select event - single selection - search not matching text', () => { + const { getAllByRole, getByTestId } = render() + + expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) + expect(getAllByRole('row')).toHaveLength(headerRow) + + expect(emitMock).toHaveBeenCalledTimes(0) + }) + + it('Does not fire select event - multiple selection - search not matching text', () => { + const { getAllByRole, getByTestId } = render() + + expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) + expect(getAllByRole('row')).toHaveLength(headerRow) + expect(emitMock).toHaveBeenCalledTimes(0) + }) + + it('Fires event - multiple selection - filter out one of two selected rows', () => { + const { getAllByRole, getByTestId } = render() + const checkboxes = getAllByRole('checkbox') + + fireEvent.click(checkboxes[1]) + fireEvent.click(checkboxes[2]) + + expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1', 'rowname2']) + expect(emitMock).toHaveBeenCalledTimes(2) + + expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'brown' } }) + expect(getAllByRole('row')).toHaveLength(headerRow + filteredItem) + + expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1']) + expect(emitMock).toHaveBeenCalledTimes(3) + }) + + it('Fires event - multiple selection - filter out none of selected rows', () => { + const { getAllByRole, getByTestId } = render() + const checkboxes = getAllByRole('checkbox') + + fireEvent.click(checkboxes[1]) + fireEvent.click(checkboxes[2]) + + expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1', 'rowname2']) + expect(emitMock).toHaveBeenCalledTimes(2) + + expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) + expect(getAllByRole('row')).toHaveLength(headerRow) + + expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', []) + expect(emitMock).toHaveBeenCalledTimes(3) + }) + + it('Does not remove selection when searching for text matching selected row', () => { + const { getAllByRole, getByTestId } = render() + const checkboxes = getAllByRole('checkbox') + + fireEvent.click(checkboxes[1]) + + expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1']) + expect(emitMock).toHaveBeenCalledTimes(1) + + expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'fox' } }) + expect(getAllByRole('row')).toHaveLength(headerRow + filteredItem) + expect(getAllByRole('checkbox')[1]).toBeChecked() + }) + it('Clicks a column - link set on second col', () => { tableProps = { ...tableProps, From 415c91ee9968fe6b265734bf0b8f5a899d13b6db Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Wed, 22 Nov 2023 10:59:32 +0100 Subject: [PATCH 3/8] fix: Do not reset table in py example when select event contains [] #2196 --- py/examples/table_events_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/examples/table_events_select.py b/py/examples/table_events_select.py index d66d489e47..9893fcd53b 100644 --- a/py/examples/table_events_select.py +++ b/py/examples/table_events_select.py @@ -7,7 +7,7 @@ @app('/demo') async def serve(q: Q): - if q.events.table and q.events.table.select: + if q.events.table and q.events.table.select is not None: q.page['description'].content = f'{q.events.table.select}' else: q.page['table'] = ui.form_card(box='1 1 3 4', items=[ From a4f5fed55ea2b13176551adbc4ad65c6b22f90a3 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Wed, 22 Nov 2023 12:10:06 +0100 Subject: [PATCH 4/8] chore: update fix comments #2196 --- ui/src/table.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ui/src/table.tsx b/ui/src/table.tsx index 28e54166b3..657896fe59 100644 --- a/ui/src/table.tsx +++ b/ui/src/table.tsx @@ -578,7 +578,8 @@ const onRenderItemColumn={onRenderItemColumn} onRenderDetailsHeader={onRenderDetailsHeader} checkboxVisibility={checkboxVisibilityMap[m.checkbox_visibility || 'on-hover']} - // Prevent selection from being cleared when reference to 'items' changes. + // Prevent selection from being cleared when 'items' are updated. + // https://github.com/microsoft/fluentui/blob/4c1cd4bbba73bbca4411db9d01ffb486b1a90303/packages/react/src/components/DetailsList/DetailsList.base.tsx#L1032. setKey='wave-table-items' /> {colContextMenuList && } @@ -969,14 +970,13 @@ export const }, [m.pagination, m.events, m.name, filter, search, currentSort, initGroups]) React.useEffect(() => { - wave.args[m.name] = [] skipNextEventEmit.current = true - // HACK: Fluent.DetailsList fires 'onSelectionChanged' event when its internal 'isAllSelected' state changes. - // When we set 'filteredItems' to [] (e.g. by searching for a string that doesn't exist in the table), - // the 'isAllSelected' changes from 'undefined' to 'false'. This fires 'onSelectionChanged' event which emits wave 'select' event. - // We don't want 'select' event to be fired in this case, so we manually set 'isAllSelected' to 'false' on the first render - // while skipping the wave 'select' event emit by setting 'skipNextEventEmit.current' to true. + // HACK: Fluent.DetailsList fires 'onSelectionChanged' event when its internal 'isAllSelected' state changes. + // Its initial 'undefined' value can be changed whether by using Select All checkbox or by setting 'items' to [] (which changes it to 'false'). + // The event emitting is undesired e.g. when we have no items selected and user searches for non-existent rows. Therefore we manually trigger it + // on the first render by setting 'isAllSelected' to 'false' while skipping the wave 'select' event emit. selection.setAllSelected(false) + wave.args[m.name] = [] if (isSingle && m.value) { selection.setKeySelected(m.value, true, false) wave.args[m.name] = [m.value] From 03756d140160aade8f31d58be280f1daebd5e446 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Thu, 30 Nov 2023 17:15:45 +0100 Subject: [PATCH 5/8] fix: emit select event only when user (de)selects row or clicks (de)select all #2196 --- ui/package-lock.json | 326 ++++++++++++++++++++++++------------------ ui/package.json | 2 +- ui/src/table.test.tsx | 65 +++------ ui/src/table.tsx | 10 +- 4 files changed, 210 insertions(+), 193 deletions(-) diff --git a/ui/package-lock.json b/ui/package-lock.json index 21b9120b59..8f266500b3 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -12,7 +12,7 @@ "@antv/g2": "^4.1.49", "@emotion/react": "^11.10.0", "@emotion/styled": "^11.10.0", - "@fluentui/react": "^8.49.0", + "@fluentui/react": "^8.58.0", "@fluentui/react-icons-mdl2": "^1.2.12", "@mui/material": "^5.9.3", "@mui/system": "^5.9.3", @@ -2389,75 +2389,83 @@ } }, "node_modules/@fluentui/date-time-utilities": { - "version": "8.2.3", - "license": "MIT", + "version": "8.5.14", + "resolved": "https://registry.npmjs.org/@fluentui/date-time-utilities/-/date-time-utilities-8.5.14.tgz", + "integrity": "sha512-Kc64ZBj0WiaSW/Bsh4fMy9oM2FIk1TgIqBV6+OgOtdKx9cXwLdmgGk8zuQTcuRnwv5WCk2M6wvW1M+eK3sNRGA==", "dependencies": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "node_modules/@fluentui/dom-utilities": { - "version": "2.1.5", - "license": "MIT", + "version": "2.2.12", + "resolved": "https://registry.npmjs.org/@fluentui/dom-utilities/-/dom-utilities-2.2.12.tgz", + "integrity": "sha512-safCKQPJTnshYG13/U2Zx1KWhOhU4vl5RAKqW7HEBfLOHds/fAR+EzTvKgO6OgxJq59JAKJvpH2QujkLXZZQ3A==", "dependencies": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "node_modules/@fluentui/font-icons-mdl2": { - "version": "8.1.20", - "license": "MIT", + "version": "8.5.27", + "resolved": "https://registry.npmjs.org/@fluentui/font-icons-mdl2/-/font-icons-mdl2-8.5.27.tgz", + "integrity": "sha512-u6J9SmdWsr3WjC7zog930IWWySA+mxLfIqfyux9oATJQPUs+76juYYbolDTJTvndVEmb+piA7qBhEubUoaXJjQ==", "dependencies": { - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, "node_modules/@fluentui/foundation-legacy": { - "version": "8.1.19", - "license": "MIT", - "dependencies": { - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/utilities": "^8.3.9", + "version": "8.2.47", + "resolved": "https://registry.npmjs.org/@fluentui/foundation-legacy/-/foundation-legacy-8.2.47.tgz", + "integrity": "sha512-El/8/makZh2fqd2YdLSTy3T2oJ3N6WCsPzkud9CdMF98Oby0jny4EAtzjBNRbAwL4/gppOYIIchVuzRL4V2rcw==", + "dependencies": { + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@fluentui/keyboard-key": { - "version": "0.3.5", - "license": "MIT", + "version": "0.4.12", + "resolved": "https://registry.npmjs.org/@fluentui/keyboard-key/-/keyboard-key-0.4.12.tgz", + "integrity": "sha512-9nPglM58ThbOEQ88KijdYl64hiTAQQ0o60HRc0vboibmr41mJ322FoBz5Q5S5QLIEbBZajrAkrDMs3PKW4CCSw==", "dependencies": { "tslib": "^2.1.0" } }, "node_modules/@fluentui/merge-styles": { - "version": "8.3.0", - "license": "MIT", + "version": "8.5.13", + "resolved": "https://registry.npmjs.org/@fluentui/merge-styles/-/merge-styles-8.5.13.tgz", + "integrity": "sha512-ocgwNlQcQwn5mNlZKFazrFVbYDEQ6BptoW4GyEv6U5TEHE8HKKYuPRf340NXCRGiacSpz3vLkyDjp+L431qUXg==", "dependencies": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "node_modules/@fluentui/react": { - "version": "8.49.0", - "license": "MIT", - "dependencies": { - "@fluentui/date-time-utilities": "^8.2.3", - "@fluentui/font-icons-mdl2": "^8.1.20", - "@fluentui/foundation-legacy": "^8.1.19", - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/react-focus": "^8.3.14", - "@fluentui/react-hooks": "^8.3.9", - "@fluentui/react-window-provider": "^2.1.6", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/theme": "^2.4.6", - "@fluentui/utilities": "^8.3.9", + "version": "8.58.0", + "resolved": "https://registry.npmjs.org/@fluentui/react/-/react-8.58.0.tgz", + "integrity": "sha512-pkmQG+iKyNwAx6URvYl/jQNZGLg1YuyE1WoKIKA6BCJyzSMj3jAZDj6YUhijyDPz7CREz+tC2WU01P1t2gnt1Q==", + "dependencies": { + "@fluentui/date-time-utilities": "^8.4.0", + "@fluentui/font-icons-mdl2": "^8.2.0", + "@fluentui/foundation-legacy": "^8.2.0", + "@fluentui/merge-styles": "^8.4.0", + "@fluentui/react-focus": "^8.4.0", + "@fluentui/react-hooks": "^8.5.0", + "@fluentui/react-window-provider": "^2.2.0", + "@fluentui/set-version": "^8.2.0", + "@fluentui/style-utilities": "^8.6.0", + "@fluentui/theme": "^2.5.0", + "@fluentui/utilities": "^8.6.0", "@microsoft/load-themed-styles": "^1.10.26", "tslib": "^2.1.0" }, @@ -2469,33 +2477,35 @@ } }, "node_modules/@fluentui/react-focus": { - "version": "8.3.14", - "license": "MIT", - "dependencies": { - "@fluentui/keyboard-key": "^0.3.5", - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/utilities": "^8.3.9", + "version": "8.8.34", + "resolved": "https://registry.npmjs.org/@fluentui/react-focus/-/react-focus-8.8.34.tgz", + "integrity": "sha512-GNi8MqQRdoIaYpiz5kWIQaX1mNzFz3X+UShezA3gohrXnkONUvrPBuFDyYgQXoqk67juEZ+oGxl2PpKjz08HCA==", + "dependencies": { + "@fluentui/keyboard-key": "^0.4.12", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@fluentui/react-hooks": { - "version": "8.3.9", - "license": "MIT", + "version": "8.6.33", + "resolved": "https://registry.npmjs.org/@fluentui/react-hooks/-/react-hooks-8.6.33.tgz", + "integrity": "sha512-3P9RA34QhhjFwHwCvfOqMDgCwvks4hgMsEGvQVTdrcya4uskxBx4FqCLzoMxkXcAJjJCiTJmPx/mZQqQpgoyoA==", "dependencies": { - "@fluentui/react-window-provider": "^2.1.6", - "@fluentui/set-version": "^8.1.5", - "@fluentui/utilities": "^8.3.9", + "@fluentui/react-window-provider": "^2.2.16", + "@fluentui/set-version": "^8.2.12", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@fluentui/react-icon-provider": { @@ -2528,62 +2538,67 @@ } }, "node_modules/@fluentui/react-window-provider": { - "version": "2.1.6", - "license": "MIT", + "version": "2.2.16", + "resolved": "https://registry.npmjs.org/@fluentui/react-window-provider/-/react-window-provider-2.2.16.tgz", + "integrity": "sha512-4gkUMSAUjo3cgCGt+0VvTbMy9qbF6zo/cmmfYtfqbSFtXz16lKixSCMIf66gXdKjovqRGVFC/XibqfrXM2QLuw==", "dependencies": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@fluentui/set-version": { - "version": "8.1.5", - "license": "MIT", + "version": "8.2.12", + "resolved": "https://registry.npmjs.org/@fluentui/set-version/-/set-version-8.2.12.tgz", + "integrity": "sha512-I4uXIg9xkL2Heotf1+7CyGcHQskdtMSH0B5mSV0TL3w7WI2qpnzrpKuP2Kq6DHZN6Xrsg4ORFNJSjLxq/s9cUQ==", "dependencies": { "tslib": "^2.1.0" } }, "node_modules/@fluentui/style-utilities": { - "version": "8.5.5", - "license": "MIT", - "dependencies": { - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", - "@fluentui/theme": "^2.4.9", - "@fluentui/utilities": "^8.4.1", + "version": "8.9.20", + "resolved": "https://registry.npmjs.org/@fluentui/style-utilities/-/style-utilities-8.9.20.tgz", + "integrity": "sha512-oj0ghn21DnqCardlcEp+zob3IEAfA/Z7ZjzuYqlHuPUItwRqGmpr1wErssRC4R1kHsH6gq9ALxVgMa4/FvdzGg==", + "dependencies": { + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/theme": "^2.6.38", + "@fluentui/utilities": "^8.13.21", "@microsoft/load-themed-styles": "^1.10.26", "tslib": "^2.1.0" } }, "node_modules/@fluentui/theme": { - "version": "2.4.9", - "license": "MIT", + "version": "2.6.38", + "resolved": "https://registry.npmjs.org/@fluentui/theme/-/theme-2.6.38.tgz", + "integrity": "sha512-LObK/mZOQFb3aTcDlKBSLpPV0BOp5BOuNqg0Wps51b1RlisI6oS3STmw3BkcAe6jOi/p4cgLpwHMkYHh2o8PmQ==", "dependencies": { - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", - "@fluentui/utilities": "^8.4.1", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@fluentui/utilities": { - "version": "8.4.1", - "license": "MIT", + "version": "8.13.21", + "resolved": "https://registry.npmjs.org/@fluentui/utilities/-/utilities-8.13.21.tgz", + "integrity": "sha512-YPWsRAL1jgbPxf+wAY8p6LjIG4em0NReqgU8ZCFnQx9wpQbe/ZRjQcaU06pD1tYtRGvyCutwhnWDaQHDw843Xg==", "dependencies": { - "@fluentui/dom-utilities": "^2.1.5", - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", + "@fluentui/dom-utilities": "^2.2.12", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" }, "peerDependencies": { - "@types/react": ">=16.8.0 <18.0.0", - "react": ">=16.8.0 <18.0.0" + "@types/react": ">=16.8.0 <19.0.0", + "react": ">=16.8.0 <19.0.0" } }, "node_modules/@formatjs/ecma402-abstract": { @@ -19687,85 +19702,104 @@ } }, "@fluentui/date-time-utilities": { - "version": "8.2.3", + "version": "8.5.14", + "resolved": "https://registry.npmjs.org/@fluentui/date-time-utilities/-/date-time-utilities-8.5.14.tgz", + "integrity": "sha512-Kc64ZBj0WiaSW/Bsh4fMy9oM2FIk1TgIqBV6+OgOtdKx9cXwLdmgGk8zuQTcuRnwv5WCk2M6wvW1M+eK3sNRGA==", "requires": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "@fluentui/dom-utilities": { - "version": "2.1.5", + "version": "2.2.12", + "resolved": "https://registry.npmjs.org/@fluentui/dom-utilities/-/dom-utilities-2.2.12.tgz", + "integrity": "sha512-safCKQPJTnshYG13/U2Zx1KWhOhU4vl5RAKqW7HEBfLOHds/fAR+EzTvKgO6OgxJq59JAKJvpH2QujkLXZZQ3A==", "requires": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "@fluentui/font-icons-mdl2": { - "version": "8.1.20", + "version": "8.5.27", + "resolved": "https://registry.npmjs.org/@fluentui/font-icons-mdl2/-/font-icons-mdl2-8.5.27.tgz", + "integrity": "sha512-u6J9SmdWsr3WjC7zog930IWWySA+mxLfIqfyux9oATJQPUs+76juYYbolDTJTvndVEmb+piA7qBhEubUoaXJjQ==", "requires": { - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, "@fluentui/foundation-legacy": { - "version": "8.1.19", - "requires": { - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/utilities": "^8.3.9", + "version": "8.2.47", + "resolved": "https://registry.npmjs.org/@fluentui/foundation-legacy/-/foundation-legacy-8.2.47.tgz", + "integrity": "sha512-El/8/makZh2fqd2YdLSTy3T2oJ3N6WCsPzkud9CdMF98Oby0jny4EAtzjBNRbAwL4/gppOYIIchVuzRL4V2rcw==", + "requires": { + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, "@fluentui/keyboard-key": { - "version": "0.3.5", + "version": "0.4.12", + "resolved": "https://registry.npmjs.org/@fluentui/keyboard-key/-/keyboard-key-0.4.12.tgz", + "integrity": "sha512-9nPglM58ThbOEQ88KijdYl64hiTAQQ0o60HRc0vboibmr41mJ322FoBz5Q5S5QLIEbBZajrAkrDMs3PKW4CCSw==", "requires": { "tslib": "^2.1.0" } }, "@fluentui/merge-styles": { - "version": "8.3.0", + "version": "8.5.13", + "resolved": "https://registry.npmjs.org/@fluentui/merge-styles/-/merge-styles-8.5.13.tgz", + "integrity": "sha512-ocgwNlQcQwn5mNlZKFazrFVbYDEQ6BptoW4GyEv6U5TEHE8HKKYuPRf340NXCRGiacSpz3vLkyDjp+L431qUXg==", "requires": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "@fluentui/react": { - "version": "8.49.0", - "requires": { - "@fluentui/date-time-utilities": "^8.2.3", - "@fluentui/font-icons-mdl2": "^8.1.20", - "@fluentui/foundation-legacy": "^8.1.19", - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/react-focus": "^8.3.14", - "@fluentui/react-hooks": "^8.3.9", - "@fluentui/react-window-provider": "^2.1.6", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/theme": "^2.4.6", - "@fluentui/utilities": "^8.3.9", + "version": "8.58.0", + "resolved": "https://registry.npmjs.org/@fluentui/react/-/react-8.58.0.tgz", + "integrity": "sha512-pkmQG+iKyNwAx6URvYl/jQNZGLg1YuyE1WoKIKA6BCJyzSMj3jAZDj6YUhijyDPz7CREz+tC2WU01P1t2gnt1Q==", + "requires": { + "@fluentui/date-time-utilities": "^8.4.0", + "@fluentui/font-icons-mdl2": "^8.2.0", + "@fluentui/foundation-legacy": "^8.2.0", + "@fluentui/merge-styles": "^8.4.0", + "@fluentui/react-focus": "^8.4.0", + "@fluentui/react-hooks": "^8.5.0", + "@fluentui/react-window-provider": "^2.2.0", + "@fluentui/set-version": "^8.2.0", + "@fluentui/style-utilities": "^8.6.0", + "@fluentui/theme": "^2.5.0", + "@fluentui/utilities": "^8.6.0", "@microsoft/load-themed-styles": "^1.10.26", "tslib": "^2.1.0" } }, "@fluentui/react-focus": { - "version": "8.3.14", - "requires": { - "@fluentui/keyboard-key": "^0.3.5", - "@fluentui/merge-styles": "^8.2.3", - "@fluentui/set-version": "^8.1.5", - "@fluentui/style-utilities": "^8.5.2", - "@fluentui/utilities": "^8.3.9", + "version": "8.8.34", + "resolved": "https://registry.npmjs.org/@fluentui/react-focus/-/react-focus-8.8.34.tgz", + "integrity": "sha512-GNi8MqQRdoIaYpiz5kWIQaX1mNzFz3X+UShezA3gohrXnkONUvrPBuFDyYgQXoqk67juEZ+oGxl2PpKjz08HCA==", + "requires": { + "@fluentui/keyboard-key": "^0.4.12", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/style-utilities": "^8.9.20", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, "@fluentui/react-hooks": { - "version": "8.3.9", + "version": "8.6.33", + "resolved": "https://registry.npmjs.org/@fluentui/react-hooks/-/react-hooks-8.6.33.tgz", + "integrity": "sha512-3P9RA34QhhjFwHwCvfOqMDgCwvks4hgMsEGvQVTdrcya4uskxBx4FqCLzoMxkXcAJjJCiTJmPx/mZQqQpgoyoA==", "requires": { - "@fluentui/react-window-provider": "^2.1.6", - "@fluentui/set-version": "^8.1.5", - "@fluentui/utilities": "^8.3.9", + "@fluentui/react-window-provider": "^2.2.16", + "@fluentui/set-version": "^8.2.12", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, @@ -19788,44 +19822,54 @@ } }, "@fluentui/react-window-provider": { - "version": "2.1.6", + "version": "2.2.16", + "resolved": "https://registry.npmjs.org/@fluentui/react-window-provider/-/react-window-provider-2.2.16.tgz", + "integrity": "sha512-4gkUMSAUjo3cgCGt+0VvTbMy9qbF6zo/cmmfYtfqbSFtXz16lKixSCMIf66gXdKjovqRGVFC/XibqfrXM2QLuw==", "requires": { - "@fluentui/set-version": "^8.1.5", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, "@fluentui/set-version": { - "version": "8.1.5", + "version": "8.2.12", + "resolved": "https://registry.npmjs.org/@fluentui/set-version/-/set-version-8.2.12.tgz", + "integrity": "sha512-I4uXIg9xkL2Heotf1+7CyGcHQskdtMSH0B5mSV0TL3w7WI2qpnzrpKuP2Kq6DHZN6Xrsg4ORFNJSjLxq/s9cUQ==", "requires": { "tslib": "^2.1.0" } }, "@fluentui/style-utilities": { - "version": "8.5.5", - "requires": { - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", - "@fluentui/theme": "^2.4.9", - "@fluentui/utilities": "^8.4.1", + "version": "8.9.20", + "resolved": "https://registry.npmjs.org/@fluentui/style-utilities/-/style-utilities-8.9.20.tgz", + "integrity": "sha512-oj0ghn21DnqCardlcEp+zob3IEAfA/Z7ZjzuYqlHuPUItwRqGmpr1wErssRC4R1kHsH6gq9ALxVgMa4/FvdzGg==", + "requires": { + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/theme": "^2.6.38", + "@fluentui/utilities": "^8.13.21", "@microsoft/load-themed-styles": "^1.10.26", "tslib": "^2.1.0" } }, "@fluentui/theme": { - "version": "2.4.9", + "version": "2.6.38", + "resolved": "https://registry.npmjs.org/@fluentui/theme/-/theme-2.6.38.tgz", + "integrity": "sha512-LObK/mZOQFb3aTcDlKBSLpPV0BOp5BOuNqg0Wps51b1RlisI6oS3STmw3BkcAe6jOi/p4cgLpwHMkYHh2o8PmQ==", "requires": { - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", - "@fluentui/utilities": "^8.4.1", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", + "@fluentui/utilities": "^8.13.21", "tslib": "^2.1.0" } }, "@fluentui/utilities": { - "version": "8.4.1", + "version": "8.13.21", + "resolved": "https://registry.npmjs.org/@fluentui/utilities/-/utilities-8.13.21.tgz", + "integrity": "sha512-YPWsRAL1jgbPxf+wAY8p6LjIG4em0NReqgU8ZCFnQx9wpQbe/ZRjQcaU06pD1tYtRGvyCutwhnWDaQHDw843Xg==", "requires": { - "@fluentui/dom-utilities": "^2.1.5", - "@fluentui/merge-styles": "^8.3.0", - "@fluentui/set-version": "^8.1.5", + "@fluentui/dom-utilities": "^2.2.12", + "@fluentui/merge-styles": "^8.5.13", + "@fluentui/set-version": "^8.2.12", "tslib": "^2.1.0" } }, diff --git a/ui/package.json b/ui/package.json index 186d16c8ea..081c288a7f 100644 --- a/ui/package.json +++ b/ui/package.json @@ -6,7 +6,7 @@ "@antv/g2": "^4.1.49", "@emotion/react": "^11.10.0", "@emotion/styled": "^11.10.0", - "@fluentui/react": "^8.49.0", + "@fluentui/react": "^8.58.0", "@fluentui/react-icons-mdl2": "^1.2.12", "@mui/material": "^5.9.3", "@mui/system": "^5.9.3", diff --git a/ui/src/table.test.tsx b/ui/src/table.test.tsx index a614c983e3..fa8fc305f7 100644 --- a/ui/src/table.test.tsx +++ b/ui/src/table.test.tsx @@ -360,59 +360,35 @@ describe('Table.tsx', () => { expect(emitMock).toHaveBeenCalledTimes(1) }) - it('Does not fire select event - single selection - search not matching text', () => { - const { getAllByRole, getByTestId } = render() - - expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) - fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) - expect(getAllByRole('row')).toHaveLength(headerRow) - - expect(emitMock).toHaveBeenCalledTimes(0) - }) - - it('Does not fire select event - multiple selection - search not matching text', () => { + it('Does not fire select event when using search', () => { const { getAllByRole, getByTestId } = render() expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) - fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) - expect(getAllByRole('row')).toHaveLength(headerRow) - expect(emitMock).toHaveBeenCalledTimes(0) - }) - - it('Fires event - multiple selection - filter out one of two selected rows', () => { - const { getAllByRole, getByTestId } = render() - const checkboxes = getAllByRole('checkbox') - - fireEvent.click(checkboxes[1]) - fireEvent.click(checkboxes[2]) - expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1', 'rowname2']) - expect(emitMock).toHaveBeenCalledTimes(2) + // Select item. + fireEvent.click(getAllByRole('checkbox')[1]) + expect(emitMock).toHaveBeenCalledTimes(1) - expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) - fireEvent.change(getByTestId('search'), { target: { value: 'brown' } }) - expect(getAllByRole('row')).toHaveLength(headerRow + filteredItem) + // Exclude selected item by using search. + fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) - expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1']) - expect(emitMock).toHaveBeenCalledTimes(3) + expect(getAllByRole('row')).toHaveLength(headerRow) + expect(emitMock).toHaveBeenCalledTimes(1) }) - it('Fires event - multiple selection - filter out none of selected rows', () => { - const { getAllByRole, getByTestId } = render() - const checkboxes = getAllByRole('checkbox') + it('Does not fire select event when using filter', () => { + const { container, getAllByRole, getAllByText } = render() - fireEvent.click(checkboxes[1]) - fireEvent.click(checkboxes[2]) - - expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1', 'rowname2']) - expect(emitMock).toHaveBeenCalledTimes(2) + // Select item. + fireEvent.click(getAllByRole('checkbox')[0]) + expect(emitMock).toHaveBeenCalledTimes(1) - expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) - fireEvent.change(getByTestId('search'), { target: { value: 'No match!' } }) - expect(getAllByRole('row')).toHaveLength(headerRow) + // Exclude selected item by using filter. + fireEvent.click(container.querySelector('.ms-DetailsHeader-filterChevron') as HTMLElement) + fireEvent.click(getAllByText('2')[1].parentElement as HTMLDivElement) - expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', []) - expect(emitMock).toHaveBeenCalledTimes(3) + expect(getAllByRole('gridcell')[1].textContent).toBe('Quick brown fox.') + expect(emitMock).toHaveBeenCalledTimes(1) }) it('Does not remove selection when searching for text matching selected row', () => { @@ -420,11 +396,8 @@ describe('Table.tsx', () => { const checkboxes = getAllByRole('checkbox') fireEvent.click(checkboxes[1]) - - expect(emitMock).toHaveBeenCalledWith(tableProps.name, 'select', ['rowname1']) - expect(emitMock).toHaveBeenCalledTimes(1) - expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) + fireEvent.change(getByTestId('search'), { target: { value: 'fox' } }) expect(getAllByRole('row')).toHaveLength(headerRow + filteredItem) expect(getAllByRole('checkbox')[1]).toBeChecked() diff --git a/ui/src/table.tsx b/ui/src/table.tsx index 657896fe59..66e741613b 100644 --- a/ui/src/table.tsx +++ b/ui/src/table.tsx @@ -931,6 +931,11 @@ export const const selectedItemKeys = selection.getSelection().map(item => item.key as S) wave.args[m.name] = selectedItemKeys if (!skipNextEventEmit.current && m.events?.includes('select')) wave.emit(m.name, 'select', selectedItemKeys) + }, + onItemsChanged: () => { + // HACK: Skip emitting 'select' event on 'items' update. + skipNextEventEmit.current = true + setTimeout(() => skipNextEventEmit.current = false) } }), [m.name, m.events]), computeHeight = () => { @@ -971,11 +976,6 @@ export const React.useEffect(() => { skipNextEventEmit.current = true - // HACK: Fluent.DetailsList fires 'onSelectionChanged' event when its internal 'isAllSelected' state changes. - // Its initial 'undefined' value can be changed whether by using Select All checkbox or by setting 'items' to [] (which changes it to 'false'). - // The event emitting is undesired e.g. when we have no items selected and user searches for non-existent rows. Therefore we manually trigger it - // on the first render by setting 'isAllSelected' to 'false' while skipping the wave 'select' event emit. - selection.setAllSelected(false) wave.args[m.name] = [] if (isSingle && m.value) { selection.setKeySelected(m.value, true, false) From a48d60d6a33dfcd8750252c44e480de2b17a9fcc Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Thu, 30 Nov 2023 17:21:28 +0100 Subject: [PATCH 6/8] chore: update py example #2196 --- py/examples/table_events_select.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/py/examples/table_events_select.py b/py/examples/table_events_select.py index 9893fcd53b..8e048f56a4 100644 --- a/py/examples/table_events_select.py +++ b/py/examples/table_events_select.py @@ -7,20 +7,21 @@ @app('/demo') async def serve(q: Q): - if q.events.table and q.events.table.select is not None: + if q.events.table and q.events.table.select: q.page['description'].content = f'{q.events.table.select}' else: q.page['table'] = ui.form_card(box='1 1 3 4', items=[ ui.table( name='table', - columns=[ui.table_column(name='text', label='Table select event')], + columns=[ui.table_column(name='text', label='Table select event', searchable=True, filterable=True)], rows=[ ui.table_row(name='row1', cells=['Row 1']), ui.table_row(name='row2', cells=['Row 2']), ui.table_row(name='row3', cells=['Row 3']) ], multiple=True, - events=['select'] + events=['select'], + groupable=True ) ]) q.page['description'] = ui.markdown_card(box='4 1 3 4', title='Selected rows', content='Nothing selected yet.') From 60f69fdc5a32e0dffeaf09482b1289929349e9e7 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Thu, 30 Nov 2023 17:22:30 +0100 Subject: [PATCH 7/8] chore: update py example #2196 --- py/examples/table_events_select.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/py/examples/table_events_select.py b/py/examples/table_events_select.py index 8e048f56a4..3ef65d079d 100644 --- a/py/examples/table_events_select.py +++ b/py/examples/table_events_select.py @@ -13,7 +13,7 @@ async def serve(q: Q): q.page['table'] = ui.form_card(box='1 1 3 4', items=[ ui.table( name='table', - columns=[ui.table_column(name='text', label='Table select event', searchable=True, filterable=True)], + columns=[ui.table_column(name='text', label='Table select event')], rows=[ ui.table_row(name='row1', cells=['Row 1']), ui.table_row(name='row2', cells=['Row 2']), @@ -21,7 +21,6 @@ async def serve(q: Q): ], multiple=True, events=['select'], - groupable=True ) ]) q.page['description'] = ui.markdown_card(box='4 1 3 4', title='Selected rows', content='Nothing selected yet.') From 474423199a08500df035cc5653edd794b4f9a121 Mon Sep 17 00:00:00 2001 From: Marek Mihok Date: Thu, 30 Nov 2023 17:23:15 +0100 Subject: [PATCH 8/8] chore: update py example #2196 --- py/examples/table_events_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/examples/table_events_select.py b/py/examples/table_events_select.py index 3ef65d079d..d66d489e47 100644 --- a/py/examples/table_events_select.py +++ b/py/examples/table_events_select.py @@ -20,7 +20,7 @@ async def serve(q: Q): ui.table_row(name='row3', cells=['Row 3']) ], multiple=True, - events=['select'], + events=['select'] ) ]) q.page['description'] = ui.markdown_card(box='4 1 3 4', title='Selected rows', content='Nothing selected yet.')