From 0048ef81e3263d0c21ddc0e53c16b8eb9a955995 Mon Sep 17 00:00:00 2001 From: Matt Gallo Date: Fri, 1 Dec 2023 16:04:23 -0500 Subject: [PATCH] fix(Datagrid): expandable/nested review fixes, custom hook for keeping expander focus (v1) (#3864) * chore(Datagrid): keep expander focus, remove feat flags * chore: update snapshot * chore: remove all padding from expanded area td --- .../__snapshots__/styles.test.js.snap | 10 ++++ .../Datagrid/Datagrid/DatagridExpandedRow.js | 18 ++++--- .../Datagrid/Datagrid/DatagridHeaderRow.js | 1 + .../Datagrid/Datagrid/DatagridRow.js | 35 +++++++++---- .../ExpandableRow/ExpandableRow.stories.js | 18 +------ .../NestedRows/NestedRows.stories.js | 22 -------- .../Datagrid/styles/_useExpandedRow.scss | 21 ++++++++ .../src/components/Datagrid/useExpandedRow.js | 13 +++-- .../Datagrid/useFocusRowExpander.js | 50 +++++++++++++++++++ .../Datagrid/useNestedRowExpander.js | 11 ++++ .../src/components/Datagrid/useNestedRows.js | 5 -- .../src/components/Datagrid/useRowExpander.js | 11 ++++ .../src/global/js/package-settings.js | 2 - 13 files changed, 147 insertions(+), 70 deletions(-) create mode 100644 packages/ibm-products/src/components/Datagrid/useFocusRowExpander.js diff --git a/packages/ibm-products/src/__tests__/__snapshots__/styles.test.js.snap b/packages/ibm-products/src/__tests__/__snapshots__/styles.test.js.snap index 60b6559d5a..5f74409a3d 100644 --- a/packages/ibm-products/src/__tests__/__snapshots__/styles.test.js.snap +++ b/packages/ibm-products/src/__tests__/__snapshots__/styles.test.js.snap @@ -2963,6 +2963,16 @@ th.c4p--datagrid__select-all-toggle-on.button { background: var(--cds-layer-hover, #e5e5e5); } +.c4p--datagrid .bx--data-table tbody tr:hover td.c4p--datagrid__expanded-row-cell-wrapper, +.c4p--datagrid .bx--data-table td.c4p--datagrid__expanded-row-cell-wrapper, +.c4p--datagrid .bx--data-table .c4p--datagrid__carbon-row-expanded td.c4p--datagrid__expandable-row-cell { + border: none; +} + +.c4p--datagrid .bx--data-table td.c4p--datagrid__expanded-row-cell-wrapper { + padding: 0; +} + .c4p--datagrid__draggable-handleStyle { display: flex; align-items: center; diff --git a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridExpandedRow.js b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridExpandedRow.js index 9ea0eee798..7ff2f93a79 100644 --- a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridExpandedRow.js +++ b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridExpandedRow.js @@ -34,14 +34,16 @@ const DatagridExpandedRow = onMouseEnter={(event) => toggleParentHoverClass(event, 'enter')} onMouseLeave={(event) => toggleParentHoverClass(event)} > -
- -
+ +
+ +
+ ); }; diff --git a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridHeaderRow.js b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridHeaderRow.js index e09ff4cd19..41e6efecaf 100644 --- a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridHeaderRow.js +++ b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridHeaderRow.js @@ -72,6 +72,7 @@ const HeaderRow = (datagridState, headRef, headerGroup) => { const handleOnMouseDownResize = (event, resizeProps) => { const { onMouseDown } = { ...resizeProps() }; + event.target.focus(); // When event.button is 2, that is a right click // and we do not want to resize if (event.button === 2 || event.ctrlKey) { diff --git a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridRow.js b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridRow.js index b47edea552..249dc93599 100644 --- a/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridRow.js +++ b/packages/ibm-products/src/components/Datagrid/Datagrid/DatagridRow.js @@ -1,11 +1,10 @@ -/* - * Licensed Materials - Property of IBM - * 5724-Q36 - * (c) Copyright IBM Corp. 2020 - * US Government Users Restricted Rights - Use, duplication or disclosure - * restricted by GSA ADP Schedule Contract with IBM Corp. +/** + * Copyright IBM Corp. 2020, 2023 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. */ -// @flow + import React from 'react'; import { DataTable, SkeletonText } from 'carbon-components-react'; import { px } from '@carbon/layout'; @@ -27,7 +26,15 @@ const rowHeights = { // eslint-disable-next-line react/prop-types const DatagridRow = (datagridState) => { - const { row, rowSize, withNestedRows, prepareRow, key } = datagridState; + const { + row, + rowSize, + withNestedRows, + prepareRow, + key, + tableId, + withExpandedRows, + } = datagridState; const getVisibleNestedRowCount = ({ isExpanded, subRows }) => { let size = 0; @@ -70,7 +77,7 @@ const DatagridRow = (datagridState) => { const focusRemover = () => { const elements = document.querySelectorAll( - `.${blockClass}__carbon-row-expanded` + `#${tableId} .${blockClass}__carbon-row-expanded` ); elements.forEach((el) => { el.classList.remove(`${blockClass}__carbon-row-expanded-hover-active`); @@ -110,6 +117,15 @@ const DatagridRow = (datagridState) => { [`${carbon.prefix}--data-table--selected`]: row.isSelected, }); + const setAdditionalRowProps = () => { + if (withNestedRows || withExpandedRows) { + return { + 'data-nested-row-id': row.id, + }; + } + return {}; + }; + return ( { onFocus={hoverHandler} onBlur={focusRemover} onKeyUp={handleOnKeyUp} + {...setAdditionalRowProps()} > {row.cells.map((cell, index) => { const cellProps = cell.getCellProps({ role: false }); diff --git a/packages/ibm-products/src/components/Datagrid/Extensions/ExpandableRow/ExpandableRow.stories.js b/packages/ibm-products/src/components/Datagrid/Extensions/ExpandableRow/ExpandableRow.stories.js index 5f5e7d4d03..3539880023 100644 --- a/packages/ibm-products/src/components/Datagrid/Extensions/ExpandableRow/ExpandableRow.stories.js +++ b/packages/ibm-products/src/components/Datagrid/Extensions/ExpandableRow/ExpandableRow.stories.js @@ -20,7 +20,6 @@ import { DatagridActions } from '../../utils/DatagridActions'; import { DatagridPagination } from '../../utils/DatagridPagination'; import { makeData } from '../../utils/makeData'; import { ARG_TYPES } from '../../utils/getArgTypes'; -import { pkg } from '../../../../settings'; import { usePrefix } from '../../../../global/js/hooks'; export default { @@ -30,13 +29,6 @@ export default { styles, docs: { page: mdx }, }, - argTypes: { - featureFlags: { - table: { - disable: true, - }, - }, - }, }; const defaultHeader = [ @@ -154,11 +146,10 @@ const sharedDatagridProps = { }; const ExpansionRenderer = ({ row, expandedContentAlign }) => { - console.log(row); const prefix = usePrefix(); return (
@@ -208,12 +199,6 @@ const ExpandedRows = ({ ...args }) => { const [data] = useState(makeData(10)); const rows = React.useMemo(() => data, [data]); - // Warnings are ordinarily silenced in storybook, add this to test. - pkg._silenceWarnings(false); - // Enable feature flag for `useExpandedRow` hook - pkg.feature['Datagrid.useExpandedRow'] = true; - pkg._silenceWarnings(true); - const datagridState = useDatagrid( { columns, @@ -256,6 +241,5 @@ export const ExpandableRowStory = prepareStory(BasicTemplateWrapper, { }, args: { ...expandableRowControlProps, - featureFlags: ['Datagrid.useExpandedRow'], }, }); diff --git a/packages/ibm-products/src/components/Datagrid/Extensions/NestedRows/NestedRows.stories.js b/packages/ibm-products/src/components/Datagrid/Extensions/NestedRows/NestedRows.stories.js index 053a1efd2b..7d3e643098 100644 --- a/packages/ibm-products/src/components/Datagrid/Extensions/NestedRows/NestedRows.stories.js +++ b/packages/ibm-products/src/components/Datagrid/Extensions/NestedRows/NestedRows.stories.js @@ -19,7 +19,6 @@ import mdx from '../../Datagrid.mdx'; import { DatagridActions } from '../../utils/DatagridActions'; import { makeData } from '../../utils/makeData'; import { ARG_TYPES } from '../../utils/getArgTypes'; -import { pkg } from '../../../../settings'; export default { title: `${getStoryTitle(Datagrid.displayName)}/Extensions/NestedRows`, @@ -28,13 +27,6 @@ export default { styles, docs: { page: mdx }, }, - argTypes: { - featureFlags: { - table: { - disable: true, - }, - }, - }, }; const defaultHeader = [ @@ -175,12 +167,6 @@ const SingleLevelNestedRows = ({ ...args }) => { useNestedRows ); - // Warnings are ordinarily silenced in storybook, add this to test - pkg._silenceWarnings(false); - // Enable feature flag for `useNestedRows` hook - pkg.feature['Datagrid.useNestedRows'] = true; - pkg._silenceWarnings(true); - return ; }; @@ -205,7 +191,6 @@ export const SingleLevelNestedRowsUsageStory = prepareStory( }, args: { ...nestedRowsControlProps, - featureFlags: ['Datagrid.useNestedRows'], }, } ); @@ -214,12 +199,6 @@ const NestedRows = ({ ...args }) => { const columns = React.useMemo(() => defaultHeader, []); const [data] = useState(makeData(10, 5, 2, 2)); - // Warnings are ordinarily silenced in storybook, add this to test - pkg._silenceWarnings(false); - // Enable feature flag for `useNestedRows` hook - pkg.feature['Datagrid.useNestedRows'] = true; - pkg._silenceWarnings(true); - const datagridState = useDatagrid( { columns, @@ -252,6 +231,5 @@ export const NestedRowsUsageStory = prepareStory(BasicTemplateWrapper, { }, args: { ...nestedRowsControlProps, - featureFlags: ['Datagrid.useNestedRows'], }, }); diff --git a/packages/ibm-products/src/components/Datagrid/styles/_useExpandedRow.scss b/packages/ibm-products/src/components/Datagrid/styles/_useExpandedRow.scss index af91dc1a26..41843fdd27 100644 --- a/packages/ibm-products/src/components/Datagrid/styles/_useExpandedRow.scss +++ b/packages/ibm-products/src/components/Datagrid/styles/_useExpandedRow.scss @@ -63,3 +63,24 @@ .#{$block-class} .#{$block-class}__expandable-row--hover td { background: $layer-hover; } + +.#{$block-class} + .#{$carbon-prefix}--data-table + tbody + tr:hover + td.#{$block-class}__expanded-row-cell-wrapper, +.#{$block-class} + .#{$carbon-prefix}--data-table + td.#{$block-class}__expanded-row-cell-wrapper, +.#{$block-class} + .#{$carbon-prefix}--data-table + .#{$block-class}__carbon-row-expanded + td.#{$block-class}__expandable-row-cell { + border: none; +} + +.#{$block-class} + .#{$carbon-prefix}--data-table + td.#{$block-class}__expanded-row-cell-wrapper { + padding: 0; +} diff --git a/packages/ibm-products/src/components/Datagrid/useExpandedRow.js b/packages/ibm-products/src/components/Datagrid/useExpandedRow.js index 57dfd7a28f..21c235ee3b 100644 --- a/packages/ibm-products/src/components/Datagrid/useExpandedRow.js +++ b/packages/ibm-products/src/components/Datagrid/useExpandedRow.js @@ -5,16 +5,11 @@ * LICENSE file in the root directory of this source tree. */ -import { useEffect, useState } from 'react'; -import { pkg } from '../../settings'; +import { useState } from 'react'; import DatagridExpandedRow from './Datagrid/DatagridExpandedRow'; import useRowExpander from './useRowExpander'; const useExpandedRow = (hooks) => { - useEffect(() => { - pkg.checkReportFeatureEnabled('Datagrid.useExpandedRow'); - }, []); - useRowExpander(hooks); const useInstance = (instance) => { const { rows, expandedContentHeight, ExpandedRowContentComponent } = @@ -29,7 +24,11 @@ const useExpandedRow = (hooks) => { expandedRowsHeight[row.index] || expandedContentHeight, RowExpansionRenderer: DatagridExpandedRow(ExpandedRowContentComponent), })); - Object.assign(instance, { rows: rowsWithExpand, setExpandedRowHeight }); + Object.assign(instance, { + rows: rowsWithExpand, + setExpandedRowHeight, + withExpandedRows: true, + }); }; hooks.useInstance.push(useInstance); }; diff --git a/packages/ibm-products/src/components/Datagrid/useFocusRowExpander.js b/packages/ibm-products/src/components/Datagrid/useFocusRowExpander.js new file mode 100644 index 0000000000..aa7cf3c68b --- /dev/null +++ b/packages/ibm-products/src/components/Datagrid/useFocusRowExpander.js @@ -0,0 +1,50 @@ +/** + * Copyright IBM Corp. 2023, 2023 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { useEffect } from 'react'; + +// Focuses the row expander after a nested/expandable row state change. +// We have to add this workaround because react-table is re-rendering the entire row +// which removes the focus from the expander and interrupts the keyboard navigation +// flow. +export const useFocusRowExpander = ({ + instance, + lastExpandedRowIndex, + blockClass, + activeElement, +}) => { + useEffect(() => { + // We need to return at this point so that the focus is not stolen from + // other interactive elements in the Datagrid + if (!activeElement.classList.contains(`${blockClass}__row-expander`)) { + return; + } + const tableId = instance?.tableId; + const rowElements = document.querySelectorAll(`#${tableId} tbody tr`); + if (lastExpandedRowIndex) { + const rowElementsArray = Array.from(rowElements); + const activeRow = rowElementsArray.filter((r) => { + if (r.getAttribute('data-nested-row-id') === lastExpandedRowIndex) { + return r; + } + return null; + }); + if (activeRow.length) { + const rowExpander = activeRow[0].querySelector( + `.${blockClass}__row-expander` + ); + rowExpander.focus(); + } + } + }, [ + instance?.tableId, + instance?.expandedRows, + lastExpandedRowIndex, + blockClass, + activeElement, + ]); +}; diff --git a/packages/ibm-products/src/components/Datagrid/useNestedRowExpander.js b/packages/ibm-products/src/components/Datagrid/useNestedRowExpander.js index 60392c3640..fa091c718d 100644 --- a/packages/ibm-products/src/components/Datagrid/useNestedRowExpander.js +++ b/packages/ibm-products/src/components/Datagrid/useNestedRowExpander.js @@ -10,13 +10,23 @@ import React, { useRef } from 'react'; import { ChevronRight16 } from '@carbon/icons-react'; import cx from 'classnames'; import { pkg, carbon } from '../../settings'; +import { useFocusRowExpander } from './useFocusRowExpander'; const blockClass = `${pkg.prefix}--datagrid`; const useNestedRowExpander = (hooks) => { const tempState = useRef(); + const lastExpandedRowIndex = useRef(); const useInstance = (instance) => { tempState.current = instance; }; + + useFocusRowExpander({ + instance: tempState?.current, + lastExpandedRowIndex: lastExpandedRowIndex?.current, + blockClass, + activeElement: document.activeElement, + }); + const visibleColumns = (columns) => { const expanderColumn = { id: 'expander', @@ -27,6 +37,7 @@ const useNestedRowExpander = (hooks) => { // Prevents `onRowClick` from being called if `useOnRowClick` is included event.stopPropagation(); row.toggleRowExpanded(); + lastExpandedRowIndex.current = row.id; }, }; const { diff --git a/packages/ibm-products/src/components/Datagrid/useNestedRows.js b/packages/ibm-products/src/components/Datagrid/useNestedRows.js index 4bfa4a17b1..e8ede42464 100644 --- a/packages/ibm-products/src/components/Datagrid/useNestedRows.js +++ b/packages/ibm-products/src/components/Datagrid/useNestedRows.js @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import { useEffect } from 'react'; import { pkg } from '../../settings'; import cx from 'classnames'; import useNestedRowExpander from './useNestedRowExpander'; @@ -13,10 +12,6 @@ import useNestedRowExpander from './useNestedRowExpander'; const blockClass = `${pkg.prefix}--datagrid`; const useNestedRows = (hooks) => { - useEffect(() => { - pkg.checkReportFeatureEnabled('Datagrid.useNestedRows'); - }, []); - useNestedRowExpander(hooks); const marginLeft = 24; diff --git a/packages/ibm-products/src/components/Datagrid/useRowExpander.js b/packages/ibm-products/src/components/Datagrid/useRowExpander.js index 3243a01b60..9d9e972db9 100644 --- a/packages/ibm-products/src/components/Datagrid/useRowExpander.js +++ b/packages/ibm-products/src/components/Datagrid/useRowExpander.js @@ -10,14 +10,24 @@ import React, { useRef } from 'react'; import { ChevronDown16, ChevronUp16 } from '@carbon/icons-react'; import cx from 'classnames'; import { pkg, carbon } from '../../settings'; +import { useFocusRowExpander } from './useFocusRowExpander'; const blockClass = `${pkg.prefix}--datagrid`; const useRowExpander = (hooks) => { const tempState = useRef(); + const lastExpandedRowIndex = useRef(); const useInstance = (instance) => { tempState.current = instance; }; + + useFocusRowExpander({ + instance: tempState?.current, + lastExpandedRowIndex: lastExpandedRowIndex?.current, + blockClass, + activeElement: document.activeElement, + }); + const visibleColumns = (columns) => { const expanderColumn = { id: 'expander', @@ -28,6 +38,7 @@ const useRowExpander = (hooks) => { // Prevents `onRowClick` from being called if `useOnRowClick` is included event.stopPropagation(); row.toggleRowExpanded(); + lastExpandedRowIndex.current = row.id; }, }; const { diff --git a/packages/ibm-products/src/global/js/package-settings.js b/packages/ibm-products/src/global/js/package-settings.js index b34ed7d042..5fcf94403c 100644 --- a/packages/ibm-products/src/global/js/package-settings.js +++ b/packages/ibm-products/src/global/js/package-settings.js @@ -96,8 +96,6 @@ const defaults = { feature: { 'a-new-feature': false, 'default-portal-target-body': true, - 'Datagrid.useExpandedRow': false, - 'Datagrid.useNestedRows': false, 'Datagrid.useInlineEdit': false, 'Datagrid.useFiltering': false, 'Datagrid.useCustomizeColumns': false,