From dce402f8e4d4cb5b3ac8c472d5f0a0613514ac1c Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Thu, 27 Feb 2025 09:50:01 +0100 Subject: [PATCH 1/3] feat: Table cell align for all cells --- pages/table/cell-permutations.page.tsx | 2 +- pages/table/permutations.page.tsx | 13 +++++- .../__snapshots__/documenter.test.ts.snap | 17 ++++++++ src/table/__tests__/table.test.tsx | 40 +++++++++++++++++++ src/table/index.tsx | 6 +++ src/table/interfaces.tsx | 8 ++++ src/table/internal.tsx | 15 +++++-- src/table/selection/selection-cell.tsx | 4 +- src/table/selection/selection-control.tsx | 10 +++-- src/table/selection/styles.scss | 5 +++ 10 files changed, 109 insertions(+), 11 deletions(-) diff --git a/pages/table/cell-permutations.page.tsx b/pages/table/cell-permutations.page.tsx index 304a3606fe..54866b00ef 100644 --- a/pages/table/cell-permutations.page.tsx +++ b/pages/table/cell-permutations.page.tsx @@ -224,7 +224,6 @@ function TableCellsDemo() { sortingField: index === 2 ? 'field-1' : index === 3 ? 'field-2' : undefined, activeSorting: index === 3, cell: item => cellRenderer.render(item), - verticalAlign: settings.verticalAlign, editConfig: settings.isEditable ? { ariaLabel: 'Edit dialog aria label', @@ -288,6 +287,7 @@ function TableCellsDemo() { stickyColumns={{ first: settings.stickyColumnsFirst, last: settings.stickyColumnsLast }} selectionType={selectionType} selectedItems={selectedItems} + cellVerticalAlign={settings.verticalAlign} empty="Empty" loading={settings.tableLoading} loadingText="Loading" diff --git a/pages/table/permutations.page.tsx b/pages/table/permutations.page.tsx index 59a95b370e..1e52155382 100644 --- a/pages/table/permutations.page.tsx +++ b/pages/table/permutations.page.tsx @@ -81,11 +81,17 @@ const VERTICAL_ALIGN_COLUMNS: TableProps.ColumnDefinition[] = [ verticalAlign: 'top', }, { - id: 'type-2', - header: 'Value', + id: 'value-top', + header: 'Value (top)', cell: item => item.text, verticalAlign: 'top', }, + { + id: 'value-middle', + header: 'Value (middle)', + cell: item => item.text, + verticalAlign: 'middle', + }, ]; /* eslint-disable react/jsx-key */ @@ -314,9 +320,12 @@ const permutations = createPermutations([ items: [createSimpleItems(3)], }, { + header: ['Vertical align'], columnDefinitions: [VERTICAL_ALIGN_COLUMNS], + cellVerticalAlign: ['top'], items: [createSimpleItems(3)], variant: [undefined, 'full-page'], + selectionType: ['multi'], }, { columnDefinitions: [ diff --git a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap index e6bcd8c3e2..e87e30f44e 100644 --- a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap +++ b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap @@ -16034,6 +16034,23 @@ add a meaningful description to the whole selection. "optional": true, "type": "TableProps.AriaLabels", }, + { + "defaultValue": "'middle'", + "description": "Determines the alignment of the content inside table cells. +This property affects all cells, including the ones in the selection column. +To target individual cells use \`columnDefinitions.verticalAlign\`.", + "inlineType": { + "name": "", + "type": "union", + "values": [ + "middle", + "top", + ], + }, + "name": "cellVerticalAlign", + "optional": true, + "type": "string", + }, { "deprecatedTag": "Custom CSS is not supported. For testing and other use cases, use [data attributes](https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes).", "description": "Adds the specified classes to the root element of the component.", diff --git a/src/table/__tests__/table.test.tsx b/src/table/__tests__/table.test.tsx index 1fe5be7867..9b33c3e8d4 100644 --- a/src/table/__tests__/table.test.tsx +++ b/src/table/__tests__/table.test.tsx @@ -3,6 +3,7 @@ import * as React from 'react'; import { act, fireEvent, render, waitFor } from '@testing-library/react'; +import * as baseComponentHooks from '../../../lib/components/internal/hooks/use-base-component'; import { useMobile } from '../../../lib/components/internal/hooks/use-mobile'; import PropertyFilter from '../../../lib/components/property-filter'; import Select from '../../../lib/components/select'; @@ -14,6 +15,8 @@ import bodyCellStyles from '../../../lib/components/table/body-cell/styles.css.j import headerCellStyles from '../../../lib/components/table/header-cell/styles.css.js'; import styles from '../../../lib/components/table/styles.css.js'; +const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); + jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({ useMobile: jest.fn(), })); @@ -549,3 +552,40 @@ test('shows and hides cell disabled reason', () => { wrapper.findEditCellButton(2, 1)!.click(); expect(createWrapper().findByClassName(popoverStyles.container)!.getElement()).toHaveTextContent('Cannot edit test2'); }); + +test('reports cellVerticalAlign and columnDefinitionsVerticalAlign correctly', () => { + const def = (id: string, verticalAlign: 'middle' | 'top') => ({ id, header: '', cell: () => null, verticalAlign }); + + render(); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'middle' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), + }, + expect.anything() + ); + + render(
); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'top' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: true }), + }, + expect.anything() + ); + + render(
); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'top' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), + }, + expect.anything() + ); +}); diff --git a/src/table/index.tsx b/src/table/index.tsx index aad355b3b8..eef8d56d31 100644 --- a/src/table/index.tsx +++ b/src/table/index.tsx @@ -22,6 +22,7 @@ const Table = React.forwardRef( selectedItems = [], variant = 'container', contentDensity = 'comfortable', + cellVerticalAlign = 'middle', firstIndex = 1, ...props }: TableProps, @@ -46,6 +47,7 @@ const Table = React.forwardRef( enableKeyboardNavigation: props.enableKeyboardNavigation, totalItemsCount: props.totalItemsCount, flowType: analyticsMetadata.flowType, + cellVerticalAlign, }, metadata: { expandableRows: !!props.expandableRows, @@ -61,6 +63,9 @@ const Table = React.forwardRef( hasInstanceIdentifier: Boolean(analyticsMetadata?.instanceIdentifier), usesVisibleColumns: !!props.visibleColumns, usesColumnDisplay: !!props.columnDisplay, + usesColumnDefinitionsVerticalAlign: props.columnDefinitions.some( + def => def.verticalAlign !== cellVerticalAlign + ), }, }, analyticsMetadata @@ -89,6 +94,7 @@ const Table = React.forwardRef( variant, contentDensity, firstIndex, + cellVerticalAlign, ...props, ...baseComponentProps, ref, diff --git a/src/table/interfaces.tsx b/src/table/interfaces.tsx index dd45feba7f..5cfdef2cb6 100644 --- a/src/table/interfaces.tsx +++ b/src/table/interfaces.tsx @@ -117,6 +117,14 @@ export interface TableProps extends BaseComponentProps { * * `verticalAlign` ('middle' | 'top') - Determines the alignment of the content in the table cell. */ columnDefinitions: ReadonlyArray>; + + /** + * Determines the alignment of the content inside table cells. + * This property affects all cells, including the ones in the selection column. + * To target individual cells use `columnDefinitions.verticalAlign`. + */ + cellVerticalAlign?: 'middle' | 'top'; + /** * Specifies the selection type (`'single' | 'multi'`). */ diff --git a/src/table/internal.tsx b/src/table/internal.tsx index 80e4063a65..68db37baea 100644 --- a/src/table/internal.tsx +++ b/src/table/internal.tsx @@ -68,7 +68,10 @@ const GRID_NAVIGATION_PAGE_SIZE = 10; const SELECTION_COLUMN_WIDTH = 54; const selectionColumnId = Symbol('selection-column-id'); -type InternalTableProps = SomeRequired, 'items' | 'selectedItems' | 'variant' | 'firstIndex'> & +type InternalTableProps = SomeRequired< + TableProps, + 'items' | 'selectedItems' | 'variant' | 'firstIndex' | 'cellVerticalAlign' +> & InternalBaseComponentProps & { __funnelSubStepProps?: InternalContainerProps['__funnelSubStepProps']; }; @@ -135,6 +138,7 @@ const InternalTable = React.forwardRef( renderLoaderLoading, renderLoaderError, renderLoaderEmpty, + cellVerticalAlign, __funnelSubStepProps, ...rest }: InternalTableProps, @@ -595,6 +599,7 @@ const InternalTable = React.forwardRef( rowIndex, itemKey: `${getTableItemKey(row.item)}`, }} + verticalAlign={cellVerticalAlign} /> )} @@ -644,7 +649,7 @@ const InternalTable = React.forwardRef( submitEdit={cellEditing.submitEdit} columnId={column.id ?? colIndex} colIndex={colIndex + colIndexOffset} - verticalAlign={column.verticalAlign} + verticalAlign={column.verticalAlign ?? cellVerticalAlign} {...cellExpandableProps} {...getAnalyticsMetadataAttribute(analyticsMetadata)} /> @@ -670,7 +675,11 @@ const InternalTable = React.forwardRef( {...rowRoleProps} > {getItemSelectionProps && ( - + )} {visibleColumnDefinitions.map((column, colIndex) => ( - {selectionControlProps ? : null} + {selectionControlProps ? ( + + ) : null} ); } diff --git a/src/table/selection/selection-control.tsx b/src/table/selection/selection-control.tsx index 2b55d80972..5991513fae 100644 --- a/src/table/selection/selection-control.tsx +++ b/src/table/selection/selection-control.tsx @@ -24,6 +24,7 @@ export interface SelectionControlProps extends SelectionProps { focusedComponent?: null | string; rowIndex?: number; itemKey?: string; + verticalAlign?: 'middle' | 'top'; } export function SelectionControl({ @@ -37,6 +38,7 @@ export function SelectionControl({ focusedComponent, rowIndex, itemKey, + verticalAlign = 'middle', ...sharedProps }: SelectionControlProps) { const controlId = useUniqueId(); @@ -45,7 +47,7 @@ export function SelectionControl({ const setShiftState = (event: KeyboardEvent | MouseEvent) => { if (isMultiSelection) { - onShiftToggle && onShiftToggle(event.shiftKey); + onShiftToggle?.(event.shiftKey); } }; @@ -65,11 +67,11 @@ export function SelectionControl({ if (isMultiSelection && !navigationActive) { if (event.keyCode === KeyCode.up) { event.preventDefault(); - onFocusUp && onFocusUp(event); + onFocusUp?.(event); } if (event.keyCode === KeyCode.down) { event.preventDefault(); - onFocusDown && onFocusDown(event); + onFocusDown?.(event); } } }; @@ -102,7 +104,7 @@ export function SelectionControl({ onMouseUp={setShiftState} onClick={handleClick} htmlFor={controlId} - className={clsx(styles.label, styles.root)} + className={clsx(styles.label, styles.root, verticalAlign === 'top' && styles['label-top'])} aria-label={ariaLabel} title={ariaLabel} {...(rowIndex !== undefined && !sharedProps.disabled diff --git a/src/table/selection/styles.scss b/src/table/selection/styles.scss index 40db6ab611..0e5218042f 100644 --- a/src/table/selection/styles.scss +++ b/src/table/selection/styles.scss @@ -24,6 +24,11 @@ border-inline-end: 0.1 * styles.$base-size solid transparent; } +.label-top { + align-items: baseline; + padding-block-start: awsui.$space-xs; +} + .stud { visibility: hidden; } From 3ac28e738d69c47a9ba4cc4384b2c831f0871832 Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Fri, 14 Mar 2025 07:30:45 +0100 Subject: [PATCH 2/3] minor changes --- .../__snapshots__/documenter.test.ts.snap | 2 +- .../__tests__/table-feature-metrics.test.tsx | 50 +++++++++++++++++++ src/table/__tests__/table.test.tsx | 40 --------------- src/table/interfaces.tsx | 2 +- 4 files changed, 52 insertions(+), 42 deletions(-) create mode 100644 src/table/__tests__/table-feature-metrics.test.tsx diff --git a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap index e87e30f44e..a3eb743bdb 100644 --- a/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap +++ b/src/__tests__/snapshot-tests/__snapshots__/documenter.test.ts.snap @@ -16038,7 +16038,7 @@ add a meaningful description to the whole selection. "defaultValue": "'middle'", "description": "Determines the alignment of the content inside table cells. This property affects all cells, including the ones in the selection column. -To target individual cells use \`columnDefinitions.verticalAlign\`.", +To target individual cells use \`columnDefinitions.verticalAlign\`, that takes precedence over \`cellVerticalAlign\`.", "inlineType": { "name": "", "type": "union", diff --git a/src/table/__tests__/table-feature-metrics.test.tsx b/src/table/__tests__/table-feature-metrics.test.tsx new file mode 100644 index 0000000000..28054613f7 --- /dev/null +++ b/src/table/__tests__/table-feature-metrics.test.tsx @@ -0,0 +1,50 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import * as React from 'react'; +import { render } from '@testing-library/react'; + +import * as baseComponentHooks from '../../../lib/components/internal/hooks/use-base-component'; +import Table from '../../../lib/components/table'; + +const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); + +jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({ + useMobile: jest.fn(), +})); + +test('reports cellVerticalAlign and columnDefinitionsVerticalAlign correctly', () => { + const def = (id: string, verticalAlign: 'middle' | 'top') => ({ id, header: '', cell: () => null, verticalAlign }); + + render(
); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'middle' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), + }, + expect.anything() + ); + + render(
); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'top' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: true }), + }, + expect.anything() + ); + + render(
); + + expect(useBaseComponentSpy).toHaveBeenCalledWith( + 'Table', + { + props: expect.objectContaining({ cellVerticalAlign: 'top' }), + metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), + }, + expect.anything() + ); +}); diff --git a/src/table/__tests__/table.test.tsx b/src/table/__tests__/table.test.tsx index 9b33c3e8d4..1fe5be7867 100644 --- a/src/table/__tests__/table.test.tsx +++ b/src/table/__tests__/table.test.tsx @@ -3,7 +3,6 @@ import * as React from 'react'; import { act, fireEvent, render, waitFor } from '@testing-library/react'; -import * as baseComponentHooks from '../../../lib/components/internal/hooks/use-base-component'; import { useMobile } from '../../../lib/components/internal/hooks/use-mobile'; import PropertyFilter from '../../../lib/components/property-filter'; import Select from '../../../lib/components/select'; @@ -15,8 +14,6 @@ import bodyCellStyles from '../../../lib/components/table/body-cell/styles.css.j import headerCellStyles from '../../../lib/components/table/header-cell/styles.css.js'; import styles from '../../../lib/components/table/styles.css.js'; -const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); - jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({ useMobile: jest.fn(), })); @@ -552,40 +549,3 @@ test('shows and hides cell disabled reason', () => { wrapper.findEditCellButton(2, 1)!.click(); expect(createWrapper().findByClassName(popoverStyles.container)!.getElement()).toHaveTextContent('Cannot edit test2'); }); - -test('reports cellVerticalAlign and columnDefinitionsVerticalAlign correctly', () => { - const def = (id: string, verticalAlign: 'middle' | 'top') => ({ id, header: '', cell: () => null, verticalAlign }); - - render(
); - - expect(useBaseComponentSpy).toHaveBeenCalledWith( - 'Table', - { - props: expect.objectContaining({ cellVerticalAlign: 'middle' }), - metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), - }, - expect.anything() - ); - - render(
); - - expect(useBaseComponentSpy).toHaveBeenCalledWith( - 'Table', - { - props: expect.objectContaining({ cellVerticalAlign: 'top' }), - metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: true }), - }, - expect.anything() - ); - - render(
); - - expect(useBaseComponentSpy).toHaveBeenCalledWith( - 'Table', - { - props: expect.objectContaining({ cellVerticalAlign: 'top' }), - metadata: expect.objectContaining({ usesColumnDefinitionsVerticalAlign: false }), - }, - expect.anything() - ); -}); diff --git a/src/table/interfaces.tsx b/src/table/interfaces.tsx index 5cfdef2cb6..09fa785c86 100644 --- a/src/table/interfaces.tsx +++ b/src/table/interfaces.tsx @@ -121,7 +121,7 @@ export interface TableProps extends BaseComponentProps { /** * Determines the alignment of the content inside table cells. * This property affects all cells, including the ones in the selection column. - * To target individual cells use `columnDefinitions.verticalAlign`. + * To target individual cells use `columnDefinitions.verticalAlign`, that takes precedence over `cellVerticalAlign`. */ cellVerticalAlign?: 'middle' | 'top'; From 36d1aba12774aba7f8e80cb09bece19f8a4133ca Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Fri, 14 Mar 2025 10:44:10 +0100 Subject: [PATCH 3/3] remove unneeded mock --- src/table/__tests__/table-feature-metrics.test.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/table/__tests__/table-feature-metrics.test.tsx b/src/table/__tests__/table-feature-metrics.test.tsx index 28054613f7..07d09a07f5 100644 --- a/src/table/__tests__/table-feature-metrics.test.tsx +++ b/src/table/__tests__/table-feature-metrics.test.tsx @@ -8,10 +8,6 @@ import Table from '../../../lib/components/table'; const useBaseComponentSpy = jest.spyOn(baseComponentHooks, 'default'); -jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({ - useMobile: jest.fn(), -})); - test('reports cellVerticalAlign and columnDefinitionsVerticalAlign correctly', () => { const def = (id: string, verticalAlign: 'middle' | 'top') => ({ id, header: '', cell: () => null, verticalAlign });