From 413e01322bf6b074f54656f43c2656261a55d27c Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Dec 2021 14:08:26 -0800 Subject: [PATCH 1/5] Add new useUpdateEffect hook - essentially just useEffect that fires only on update/rerender and not on mount - inspiration from https://github.com/streamich/react-use/blob/master/docs/useUpdateEffect.md + update useDependentState to dogfood hook (since it's essentially using the same ref/mount logic that I want) --- src/services/hooks/index.ts | 1 + src/services/hooks/useDependentState.ts | 20 ++----- src/services/hooks/useUpdateEffect.test.tsx | 58 +++++++++++++++++++++ src/services/hooks/useUpdateEffect.ts | 25 +++++++++ src/services/index.ts | 1 + 5 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 src/services/hooks/useUpdateEffect.test.tsx create mode 100644 src/services/hooks/useUpdateEffect.ts diff --git a/src/services/hooks/index.ts b/src/services/hooks/index.ts index 7c6224ff242..573f9165bb2 100644 --- a/src/services/hooks/index.ts +++ b/src/services/hooks/index.ts @@ -7,6 +7,7 @@ */ export * from './useCombinedRefs'; +export * from './useUpdateEffect'; export * from './useDependentState'; export * from './useIsWithinBreakpoints'; export * from './useMouseMove'; diff --git a/src/services/hooks/useDependentState.ts b/src/services/hooks/useDependentState.ts index 84db057d2f0..9eb30d541af 100644 --- a/src/services/hooks/useDependentState.ts +++ b/src/services/hooks/useDependentState.ts @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import { useEffect, useState, useRef } from 'react'; +import { useState } from 'react'; +import { useUpdateEffect } from './useUpdateEffect'; export function useDependentState( valueFn: (previousState: undefined | T) => T, @@ -14,20 +15,9 @@ export function useDependentState( ) { const [state, setState] = useState(valueFn as () => T); - // use ref instead of a state to avoid causing an unnecessary re-render - const hasMounted = useRef(false); - - useEffect(() => { - // don't call setState on initial mount - if (hasMounted.current === true) { - setState(valueFn); - } else { - hasMounted.current = true; - } - - // purposefully omitting `updateCount.current` and `valueFn` - // this means updating only the valueFn has no effect, but allows for more natural feeling hook use - // eslint-disable-next-line react-hooks/exhaustive-deps + // don't call setState on initial mount + useUpdateEffect(() => { + setState(valueFn); }, deps); return [state, setState] as const; diff --git a/src/services/hooks/useUpdateEffect.test.tsx b/src/services/hooks/useUpdateEffect.test.tsx new file mode 100644 index 00000000000..2599918629c --- /dev/null +++ b/src/services/hooks/useUpdateEffect.test.tsx @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { mount } from 'enzyme'; +import { useUpdateEffect } from './useUpdateEffect'; + +describe('useUpdateEffect', () => { + const mockEffect = jest.fn(); + const mockCleanup = jest.fn(); + + const MockComponent = ({ test }: { test?: boolean }) => { + useUpdateEffect(() => { + mockEffect(); + return () => mockCleanup(); + }, [test]); + + return null; + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('does not invoke the passed effect on initial mount', () => { + mount(); + + expect(mockEffect).not.toHaveBeenCalled(); + }); + + it('invokes the passed effect on each component update/rerender', () => { + const component = mount(); + + component.setProps({ test: true }); + expect(mockEffect).toHaveBeenCalledTimes(1); + + component.setProps({ test: false }); + expect(mockEffect).toHaveBeenCalledTimes(2); + + component.setProps({ test: true }); + expect(mockEffect).toHaveBeenCalledTimes(3); + }); + + it('invokes returned cleanup, same as useEffect', () => { + const component = mount(); + + component.setProps({ test: true }); // Trigger first update/call + expect(mockCleanup).not.toHaveBeenCalled(); + + component.unmount(); // Trigger cleanup + expect(mockCleanup).toHaveBeenCalled(); + }); +}); diff --git a/src/services/hooks/useUpdateEffect.ts b/src/services/hooks/useUpdateEffect.ts new file mode 100644 index 00000000000..24a708a223e --- /dev/null +++ b/src/services/hooks/useUpdateEffect.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { useEffect, useRef } from 'react'; + +export const useUpdateEffect = (effect: Function, deps: unknown[]) => { + // use ref instead of a state to avoid causing an unnecessary re-render + const hasMounted = useRef(false); + + useEffect(() => { + // don't invoke the effect on initial mount + if (hasMounted.current === true) { + return effect(); + } else { + hasMounted.current = true; + } + + // eslint-disable-next-line react-hooks/exhaustive-deps + }, deps); +}; diff --git a/src/services/index.ts b/src/services/index.ts index a0d5677f501..88d92de726c 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -125,6 +125,7 @@ export { EuiWindowEvent } from './window_event'; export { useCombinedRefs, + useUpdateEffect, useDependentState, useIsWithinBreakpoints, useMouseMove, From fb5bb6326c3425d5d702967f5ef1320fd31f43ea Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Dec 2021 14:13:51 -0800 Subject: [PATCH 2/5] Add `onChange` callbacks to `gridStyles` and `rowHeightsOptions` - and fire said callbacks on update whenever user configurations change --- .../controls/display_selector.test.tsx | 37 ++++++++++++++++++- .../datagrid/controls/display_selector.tsx | 12 ++++++ src/components/datagrid/data_grid_types.ts | 10 +++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/components/datagrid/controls/display_selector.test.tsx b/src/components/datagrid/controls/display_selector.test.tsx index 88b699a8d01..798b01c72fa 100644 --- a/src/components/datagrid/controls/display_selector.test.tsx +++ b/src/components/datagrid/controls/display_selector.test.tsx @@ -92,6 +92,24 @@ describe('useDataGridDisplaySelector', () => { ).toEqual('tableDensityCompact'); }); + it('calls the gridStyles.onDensityChange callback on user change', () => { + const onDensityChange = jest.fn(); + const component = mount( + + ); + + openPopover(component); + component.find('[data-test-subj="expanded"]').simulate('change'); + + expect(onDensityChange).toHaveBeenCalledWith({ + stripes: true, + fontSize: 'l', + cellPadding: 'l', + }); + }); + it('hides the density buttongroup if allowDensity is set to false', () => { const component = mount( @@ -153,6 +171,23 @@ describe('useDataGridDisplaySelector', () => { expect(getSelection(component)).toEqual('auto'); }); + it('calls the rowHeightsOptions.onChange callback on user change', () => { + const onRowHeightChange = jest.fn(); + const component = mount( + + ); + + openPopover(component); + component.find('[data-test-subj="auto"]').simulate('change'); + + expect(onRowHeightChange).toHaveBeenCalledWith({ + defaultHeight: 'auto', + lineHeight: '3', + }); + }); + it('hides the row height buttongroup if allowRowHeight is set to false', () => { const component = mount( @@ -286,7 +321,7 @@ describe('useDataGridDisplaySelector', () => { it('returns an object of grid styles with user overrides', () => { const initialStyles = { ...startingStyles, stripes: true }; const MockComponent = () => { - const [, gridStyles] = useDataGridDisplaySelector( + const [, { onChange, ...gridStyles }] = useDataGridDisplaySelector( true, initialStyles, {} diff --git a/src/components/datagrid/controls/display_selector.tsx b/src/components/datagrid/controls/display_selector.tsx index bafc4591c46..7c5a25ee890 100644 --- a/src/components/datagrid/controls/display_selector.tsx +++ b/src/components/datagrid/controls/display_selector.tsx @@ -8,6 +8,7 @@ import React, { ReactNode, useState, useMemo, useCallback } from 'react'; +import { useUpdateEffect } from '../../../services'; import { EuiI18n, useEuiI18n } from '../../i18n'; import { EuiPopover } from '../../popover'; import { EuiButtonIcon, EuiButtonGroup } from '../../button'; @@ -163,6 +164,17 @@ export const useDataGridDisplaySelector = ( }; }, [initialRowHeightsOptions, userRowHeightsOptions]); + // Invoke onChange callbacks on user input (removing the callback value itself, so that only configuration values are returned) + useUpdateEffect(() => { + const { onChange, ...currentGridStyles } = gridStyles; + initialStyles?.onChange?.(currentGridStyles); + }, [userGridStyles]); + + useUpdateEffect(() => { + const { onChange, ...currentRowHeightsOptions } = rowHeightsOptions; + initialRowHeightsOptions?.onChange?.(currentRowHeightsOptions); + }, [userRowHeightsOptions]); + const buttonLabel = useEuiI18n( 'euiDisplaySelector.buttonText', 'Display options' diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts index 9b1b15ab9e8..103898dfc26 100644 --- a/src/components/datagrid/data_grid_types.ts +++ b/src/components/datagrid/data_grid_types.ts @@ -576,6 +576,11 @@ export interface EuiDataGridStyle { * If set to true, the footer row will be sticky */ stickyFooter?: boolean; + /** + * Optional callback returning the current `gridStyle` config when changes occur from user input (e.g. toolbar display controls). + * Can be used for, e.g. storing user `gridStyle` in a local storage object. + */ + onChange?: (gridStyle: EuiDataGridStyle) => void; } export interface EuiDataGridToolBarVisibilityColumnSelectorOptions { @@ -767,6 +772,11 @@ export interface EuiDataGridRowHeightsOptions { * Defines a global lineHeight style to apply to all cells */ lineHeight?: string; + /** + * Optional callback returning the current `rowHeightsOptions` when changes occur from user input (e.g. toolbar display controls). + * Can be used for, e.g. storing user `rowHeightsOptions` in a local storage object. + */ + onChange?: (rowHeightsOptions: EuiDataGridRowHeightsOptions) => void; } export interface EuiDataGridRowManager { From 6b1ba90bf58f075069b33810fb1ac185ffd66a42 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Dec 2021 14:17:35 -0800 Subject: [PATCH 3/5] Add documentation example - with density responsiveness & localStorage --- .../datagrid_height_options_example.js | 23 ++++- .../datagrid/datagrid_styling_example.js | 29 ++++++ .../src/views/datagrid/display_callbacks.js | 92 +++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 src-docs/src/views/datagrid/display_callbacks.js diff --git a/src-docs/src/views/datagrid/datagrid_height_options_example.js b/src-docs/src/views/datagrid/datagrid_height_options_example.js index fcaf9d43aa8..f30ba451375 100644 --- a/src-docs/src/views/datagrid/datagrid_height_options_example.js +++ b/src-docs/src/views/datagrid/datagrid_height_options_example.js @@ -1,4 +1,5 @@ import React, { Fragment } from 'react'; +import { Link } from 'react-router-dom'; import { GuideSectionTypes } from '../../components'; import { @@ -136,8 +137,8 @@ export const DataGridRowHeightOptionsExample = { By default, all rows get a height of 34 pixels, but there are scenarios where you might want to adjust the height to fit more content. To do that, you can pass an object to the{' '} - rowHeightsOptions prop. This object accepts three - properties: + rowHeightsOptions prop. This object accepts the + following properties:

  • @@ -173,6 +174,24 @@ export const DataGridRowHeightOptionsExample = {
+
  • + onChange +
      +
    • + Optional callback when the user changes the data grid's + internal rowHeightsOptions (e.g., via the + toolbar display selector). +
      • +
    • + Can be used to store and preserve user display preferences on + page refresh - see this{' '} + + data grid styling and control example + + . +
      • +
    +
    • diff --git a/src-docs/src/views/datagrid/datagrid_styling_example.js b/src-docs/src/views/datagrid/datagrid_styling_example.js index 971f14a2fdb..97494c23f2a 100644 --- a/src-docs/src/views/datagrid/datagrid_styling_example.js +++ b/src-docs/src/views/datagrid/datagrid_styling_example.js @@ -11,6 +11,9 @@ import { EuiListGroupItem, } from '../../../../src/components'; +import DataGridDisplayCallbacks from './display_callbacks'; +const dataGridDisplayCallbacksSource = require('!!raw-loader!./display_callbacks'); + import DataGridContainer from './container'; const dataGridContainerSource = require('!!raw-loader!./container'); const dataGridContainerHtml = renderToHtml(DataGridContainer); @@ -220,6 +223,32 @@ export const DataGridStylingExample = { }, demo: , }, + { + source: [ + { + type: GuideSectionTypes.JS, + code: dataGridDisplayCallbacksSource, + }, + ], + title: 'Adjusting your grid to user/toolbar changes', + text: ( + <> +

    + You can use the optional gridStyle.onChange and{' '} + rowHeightsOptions.onChange callbacks to adjust + your data grid based on user density or row height changes. +

    +

    + For example, if the user changes the grid density to compressed, you + may want to adjust a cell's content sizing in response. Or you + could store user settings in localStorage or other database to + preserve display settings on page refresh, like the below example + does. +

    + + ), + demo: , + }, { source: [ { diff --git a/src-docs/src/views/datagrid/display_callbacks.js b/src-docs/src/views/datagrid/display_callbacks.js new file mode 100644 index 00000000000..7ce872e1325 --- /dev/null +++ b/src-docs/src/views/datagrid/display_callbacks.js @@ -0,0 +1,92 @@ +import React, { useState, useCallback, useMemo } from 'react'; +import { fake } from 'faker'; + +import { EuiDataGrid, EuiIcon } from '../../../../src/components/'; + +const columns = [ + { id: 'name' }, + { id: 'email' }, + { id: 'city' }, + { id: 'country' }, + { id: 'account' }, +]; +const data = []; +for (let i = 1; i <= 5; i++) { + data.push({ + name: fake('{{name.lastName}}, {{name.firstName}} {{name.suffix}}'), + email: fake('{{internet.email}}'), + city: fake('{{address.city}}'), + country: fake('{{address.country}}'), + account: fake('{{finance.account}}'), + }); +} + +const GRID_STYLES_KEY = 'euiDataGridStyles'; +const INITIAL_STYLES = JSON.stringify({ stripes: true }); +const storedGridStyles = JSON.parse( + localStorage.getItem(GRID_STYLES_KEY) || INITIAL_STYLES +); + +const ROW_HEIGHTS_KEY = 'euiDataGridRowHeightsOptions'; +const INITIAL_ROW_HEIGHTS = JSON.stringify({}); +const storedRowHeightsOptions = JSON.parse( + localStorage.getItem(ROW_HEIGHTS_KEY) || INITIAL_ROW_HEIGHTS +); + +export default () => { + const [densitySize, setDensitySize] = useState(''); + const responsiveIcon = useCallback( + () => , + [densitySize] + ); + const responsiveIconWidth = useMemo(() => { + if (densitySize === 'l') return 44; + if (densitySize === 's') return 24; + return 32; + }, [densitySize]); + const leadingControlColumns = useMemo( + () => [ + { + id: 'icon', + width: responsiveIconWidth, + headerCellRender: responsiveIcon, + rowCellRender: responsiveIcon, + }, + ], + [responsiveIcon, responsiveIconWidth] + ); + + const saveRowHeightsOptions = useCallback((updatedRowHeights) => { + console.log(updatedRowHeights); + localStorage.setItem(ROW_HEIGHTS_KEY, JSON.stringify(updatedRowHeights)); + }, []); + + const saveGridStyles = useCallback((updatedStyles) => { + console.log(updatedStyles); + localStorage.setItem(GRID_STYLES_KEY, JSON.stringify(updatedStyles)); + setDensitySize(updatedStyles.fontSize); + }, []); + + const [visibleColumns, setVisibleColumns] = useState(() => + columns.map(({ id }) => id) + ); + + return ( + data[rowIndex][columnId]} + /> + ); +}; From f693f9c352067b79169e369d32f3425210f338b1 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Dec 2021 14:17:55 -0800 Subject: [PATCH 4/5] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d061a393bfa..291d57df543 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Updated `EuiDataGrid`'s full screen mode to use the `fullScreenExit` icon ([#5415](https://github.com/elastic/eui/pull/5415)) - Added `left.append` and `left.prepend` to `EuiDataGrid`'s `toolbarVisibility.additionalControls` prop [#5394](https://github.com/elastic/eui/pull/5394)) - Added a row height control to `EuiDataGrid`'s toolbar ([#5372](https://github.com/elastic/eui/pull/5372)) +- Added `onChange` callbacks to `EuiDataGrid`'s `gridStyle` and `rowHeightOptions` settings ([#5424](https://github.com/elastic/eui/pull/5424)) **Bug fixes** From bd98122986579f44a5b9c2360ee95eac40cb881c Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 2 Dec 2021 08:38:24 -0800 Subject: [PATCH 5/5] [PR feedback] Adjust callback demo to load stored configs on SPA navigation --- .../src/views/datagrid/display_callbacks.js | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src-docs/src/views/datagrid/display_callbacks.js b/src-docs/src/views/datagrid/display_callbacks.js index 7ce872e1325..e3491d576e2 100644 --- a/src-docs/src/views/datagrid/display_callbacks.js +++ b/src-docs/src/views/datagrid/display_callbacks.js @@ -23,15 +23,9 @@ for (let i = 1; i <= 5; i++) { const GRID_STYLES_KEY = 'euiDataGridStyles'; const INITIAL_STYLES = JSON.stringify({ stripes: true }); -const storedGridStyles = JSON.parse( - localStorage.getItem(GRID_STYLES_KEY) || INITIAL_STYLES -); const ROW_HEIGHTS_KEY = 'euiDataGridRowHeightsOptions'; const INITIAL_ROW_HEIGHTS = JSON.stringify({}); -const storedRowHeightsOptions = JSON.parse( - localStorage.getItem(ROW_HEIGHTS_KEY) || INITIAL_ROW_HEIGHTS -); export default () => { const [densitySize, setDensitySize] = useState(''); @@ -56,12 +50,21 @@ export default () => { [responsiveIcon, responsiveIconWidth] ); - const saveRowHeightsOptions = useCallback((updatedRowHeights) => { + const storedRowHeightsOptions = useMemo( + () => + JSON.parse(localStorage.getItem(ROW_HEIGHTS_KEY) || INITIAL_ROW_HEIGHTS), + [] + ); + const storeRowHeightsOptions = useCallback((updatedRowHeights) => { console.log(updatedRowHeights); localStorage.setItem(ROW_HEIGHTS_KEY, JSON.stringify(updatedRowHeights)); }, []); - const saveGridStyles = useCallback((updatedStyles) => { + const storedGridStyles = useMemo( + () => JSON.parse(localStorage.getItem(GRID_STYLES_KEY) || INITIAL_STYLES), + [] + ); + const storeGridStyles = useCallback((updatedStyles) => { console.log(updatedStyles); localStorage.setItem(GRID_STYLES_KEY, JSON.stringify(updatedStyles)); setDensitySize(updatedStyles.fontSize); @@ -77,11 +80,11 @@ export default () => { leadingControlColumns={leadingControlColumns} rowHeightsOptions={{ ...storedRowHeightsOptions, - onChange: saveRowHeightsOptions, + onChange: storeRowHeightsOptions, }} gridStyle={{ ...storedGridStyles, - onChange: saveGridStyles, + onChange: storeGridStyles, }} columns={columns} columnVisibility={{ visibleColumns, setVisibleColumns }}