Skip to content

Commit

Permalink
fix: Allows table cells be expandable and editable at the same time
Browse files Browse the repository at this point in the history
  • Loading branch information
pan-kot committed Jan 17, 2025
1 parent 89ffd99 commit a93a8b2
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 31 deletions.
5 changes: 2 additions & 3 deletions src/table/__tests__/expandable-rows.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ describe('Expandable rows', () => {
expect(table.findEditCellButton(1, 2)).not.toBe(null);
});

test('first column of expandable table cannot be editable', () => {
test('columns can be both editable and expandable', () => {
const { table } = renderTable({
items: simpleItems,
columnDefinitions: [
Expand All @@ -297,7 +297,6 @@ describe('Expandable rows', () => {
onExpandableItemToggle: () => {},
},
});
expect(table.findEditCellButton(1, 1)).toBe(null);
expect(table.findEditCellButton(1, 2)).not.toBe(null);
expect(table.findEditCellButton(1, 1)).not.toBe(null);
});
});
17 changes: 10 additions & 7 deletions src/table/body-cell/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useEffect, useRef, useState } from 'react';
import clsx from 'clsx';

import { useInternalI18n } from '../../i18n/context';
import Icon from '../../icon/internal';
Expand Down Expand Up @@ -46,6 +47,8 @@ function TableCellEditable<ItemType>({
};
const isFocusMoveNeededRef = useRef(false);

const isExpandable = rest.level !== undefined;

useEffect(() => {
if (!isEditing && editActivateRef.current && isFocusMoveNeededRef.current) {
isFocusMoveNeededRef.current = false;
Expand Down Expand Up @@ -81,9 +84,11 @@ function TableCellEditable<ItemType>({
nativeAttributes={tdNativeAttributes as TableTdElementProps['nativeAttributes']}
isEditing={isEditing}
hasSuccessIcon={showSuccessIcon && showIcon}
onClick={!isEditing ? onEditStart : undefined}
onClick={!isEditing && !isExpandable ? onEditStart : undefined}
onMouseEnter={() => setHasHover(true)}
onMouseLeave={() => setHasHover(false)}
onFocus={() => setHasFocus(true)}
onBlur={() => setHasFocus(false)}
>
{isEditing ? (
<InlineEditor
Expand Down Expand Up @@ -123,11 +128,10 @@ function TableCellEditable<ItemType>({

<div className={styles['body-cell-editor-wrapper']}>
<button
className={styles['body-cell-editor']}
className={clsx(styles['body-cell-editor'], isExpandable && styles['body-cell-editor-focusable'])}
aria-label={ariaLabels?.activateEditLabel?.(column, item)}
ref={editActivateRef}
onFocus={() => setHasFocus(true)}
onBlur={() => setHasFocus(false)}
onClick={!isEditing && isExpandable ? onEditStart : undefined}
tabIndex={editActivateTabIndex}
>
{showIcon && <Icon name="edit" />}
Expand All @@ -140,15 +144,14 @@ function TableCellEditable<ItemType>({
}

export function TableBodyCell<ItemType>(props: TableBodyCellProps<ItemType>) {
const isExpandableColumnCell = props.level !== undefined;
const editDisabledReason = props.column.editConfig?.disabledReason?.(props.item);

// Inline editing is deactivated for expandable column because editable cells are interactive
// and cannot include interactive content such as expand toggles.
if (editDisabledReason && !isExpandableColumnCell) {
if (editDisabledReason) {
return <DisabledInlineEditor editDisabledReason={editDisabledReason} {...props} />;
}
if ((props.isEditable || props.isEditing) && !isExpandableColumnCell) {
if (props.isEditable || props.isEditing) {
return <TableCellEditable {...props} />;
}

Expand Down
44 changes: 26 additions & 18 deletions src/table/body-cell/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ $cell-negative-space-vertical: 2px;
}
}
@mixin body-cell-active-hover-padding($padding-start) {
&:not(.body-cell-edit-active).body-cell-editable:hover {
&:not(.body-cell-edit-active):not(.body-cell-expandable).body-cell-editable:hover {
@include cell-padding-inline-start(calc(#{$padding-start} + #{awsui.$border-divider-list-width}));
}
}
Expand Down Expand Up @@ -358,8 +358,8 @@ $cell-negative-space-vertical: 2px;
background: 0;
border-block: 0;
border-inline: 0;
padding-block: 0;
padding-inline: 0;
padding-block: awsui.$space-scaled-xxs;
padding-inline: awsui.$space-scaled-xxs;

// This gives the editor button a small area even when the icon is not rendered.
// That is to allow programmatic interaction in tests.
Expand Down Expand Up @@ -394,6 +394,12 @@ $cell-negative-space-vertical: 2px;
// 6 space-xxs: 2 * icon left padding + 2 * icon right padding + space between icons + space between icons and editor
max-inline-size: calc(100% - 6 * #{awsui.$space-xxs} - 2 * #{awsui.$size-icon-normal});
}

&-focusable {
@include focus-visible.when-visible {
@include styles.focus-highlight(awsui.$space-button-focus-outline-gutter);
}
}
}

&.body-cell-expandable {
Expand Down Expand Up @@ -422,8 +428,6 @@ $cell-negative-space-vertical: 2px;
}

&:not(.body-cell-edit-active) {
cursor: pointer;

// Include interactive padding even when a cell is not hovered to prevent jittering when resizableColumns=false.
&:not(.resizable-columns) {
@include cell-padding-inline-end($interactive-column-padding-inline-end);
Expand All @@ -444,13 +448,13 @@ $cell-negative-space-vertical: 2px;
opacity: 0;
}

// Showing focus outline for the cell.
// We don't use our focus-visible polyfill here because it doesn't work properly with screen readers.
// These edit buttons are special because they are visually hidden (opacity: 0), but exposed to assistive technology.
// It's therefore important to display the focus outline, even when a keyboard use wasn't detected.
// For example, when an edit button is selected from the VoiceOver rotor menu.
&:focus-within {
@include cell-focus-outline;
// The editable cells are interactive but the actual focus lands on the edit button which is decorative.
// That is why we use focus-within to detect if the focus is on the edit button to draw the outline around the cell.
// For expandable+editable cells the edit button works as a normal button because the cell itself is not interactive.
&:not(.body-cell-expandable) {
&:focus-within {
@include cell-focus-outline;
}
}

&:focus-within:focus-within,
Expand All @@ -470,10 +474,14 @@ $cell-negative-space-vertical: 2px;
&:hover:hover {
position: relative;

background-color: awsui.$color-background-dropdown-item-hover;
border-block: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
border-inline: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
inset-inline: calc(-1 * #{awsui.$border-divider-list-width});
&:not(.body-cell-expandable) {
cursor: pointer;
background-color: awsui.$color-background-dropdown-item-hover;
border-block: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
border-inline: awsui.$border-divider-list-width solid awsui.$color-border-editable-cell-hover;
inset-inline: calc(-1 * #{awsui.$border-divider-list-width});
}

&.sticky-cell {
position: sticky;
}
Expand Down Expand Up @@ -504,12 +512,12 @@ $cell-negative-space-vertical: 2px;
&.body-cell-next-selected {
@include cell-padding-block(calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width} / 2)));
}
&.body-cell-last-row:not(.body-cell-selected) {
&.body-cell-last-row:not(.body-cell-expandable):not(.body-cell-selected) {
@include cell-padding-block-start(
calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width}))
);
}
&.body-cell-first-row:not(.body-cell-selected) {
&.body-cell-first-row:not(.body-cell-expandable):not(.body-cell-selected) {
@include cell-padding-block(calc(#{$cell-vertical-padding} - calc(#{awsui.$border-divider-list-width})));
}
@include focused-editor-styles;
Expand Down
12 changes: 9 additions & 3 deletions src/table/body-cell/td-element.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export interface TableTdElementProps {
onClick?: () => void;
onMouseEnter?: () => void;
onMouseLeave?: () => void;
onFocus?: () => void;
onBlur?: () => void;
children?: React.ReactNode;
isEvenRow?: boolean;
stripedRows?: boolean;
Expand Down Expand Up @@ -73,6 +75,8 @@ export const TableTdElement = React.forwardRef<HTMLTableCellElement, TableTdElem
onClick,
onMouseEnter,
onMouseLeave,
onFocus,
onBlur,
isEvenRow,
stripedRows,
isSelection,
Expand Down Expand Up @@ -137,19 +141,21 @@ export const TableTdElement = React.forwardRef<HTMLTableCellElement, TableTdElem
isEditing && !isEditingDisabled && styles['body-cell-edit-active'],
isEditing && isEditingDisabled && styles['body-cell-edit-disabled-popover'],
hasSuccessIcon && styles['body-cell-has-success'],
level !== undefined && styles['body-cell-expandable'],
level !== undefined && styles[`expandable-level-${getLevelClassSuffix(level)}`],
level !== undefined && !isEditing && styles['body-cell-expandable'],
level !== undefined && !isEditing && styles[`expandable-level-${getLevelClassSuffix(level)}`],
stickyStyles.className
)}
onClick={onClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onFocus={onFocus}
onBlur={onBlur}
ref={mergedRef}
{...nativeAttributes}
tabIndex={cellTabIndex === -1 ? undefined : cellTabIndex}
{...copyAnalyticsMetadataAttribute(rest)}
>
{level !== undefined && isExpandable && (
{level !== undefined && isExpandable && !isEditing && (
<div className={styles['expandable-toggle-wrapper']}>
<ExpandToggleButton
isExpanded={isExpanded}
Expand Down

0 comments on commit a93a8b2

Please sign in to comment.