[terra-table] New Terra table #3917

kenk2 · 2023-09-20T14:40:42Z

Summary

What was changed:
Implementing new Terra Table. Terra Outline Table is deleted.

Why it was changed:
Implementing new table for datagrid initiatives. Terra Outline Table is obsolete and not being worked on.

Testing

This change was tested using:

Reviews

In addition to engineering reviews, this PR needs:

UX review
Accessibility review
Functional review

Additional Details

This PR resolves:

UXPLATFORM-XXXX

Thank you for contributing to Terra.
@cerner/terra

…erra-table.

…background color.

…e obsolete properties.

…lumns.

…o table for enabling column selections and callbacks. Add more examples for table.

packages/terra-core-docs/package.json

…Fix some behavior with cells focusing after navigation.

…ble tables more interactable. Removed un-necessary ref initialization. Remove table container to handle scroll from consumers. Light refactor of pinned columns logic.

… focusing by keyboard.

smason0 · 2023-10-11T14:54:06Z

packages/terra-table/src/Table.jsx

+  columnHeaderHeight: '2.5rem',
+  rowHeight: '2.5rem',
+  pinnedColumns: [],
+  overflowColumns: [],


Should we add a default empty array for rows as well?

Right now I'm requiring rows to be supplied (but it can be empty) since in DataGrid this is actually a bug right now (component will not render otherwise). But I could also see a case where we might want to default to [] if rows is not specified.

I might suggest also making it optional if we allow rows to be empty, as that puts less burden on the consumer. If we leave it like it is here, we'd need to make sure the component doesn't completely break when nothing is provided for rows, as is happening with DataGrid.

On second thought you're right, and after thinking about it the pattern does feel awkward to pass in an empty array.

Done.

smason0 · 2023-10-11T15:05:01Z

packages/terra-table/src/subcomponents/Cell.jsx

+  isMasked: false,
+};
+
+function Cell(props) {


So just wanting to make sure, this package will be where all these subcomponents will live instead of in data-grid? So we plan on removing them from data-grid when consuming Table? I have some in-flight changes to both Row and Cell as part of Flowsheet that, depending on the PR review, I would need to make sure don't get lost in the process of migrating these.

In the future, yes these components will all live inside this Table package. If you finish first, then I'll be adding your changes into this PR. If I finish first with this then we'll have to make another PR to add in-fight work to this PR when it happens. But most of it should be copy-paste and not require significant rework.

Disregard, I'm changing my code to no longer affect Cell or Row.

sdadn · 2023-10-11T15:41:33Z

packages/terra-core-docs/package.json

@@ -36,6 +36,7 @@
  },
  "dependencies": {
    "@cerner/terra-docs": "^1.9.0",
+    "legacy-terra-table": "npm:terra-table@^4.36.0",


nice use of an alias

sdadn · 2023-10-11T15:44:06Z

packages/terra-core-docs/src/terra-dev-site/doc/table/UpgradeGuide.1.doc.mdx

+
+## Changes from 4.x to 5.0
+
+Terra table will no longer support grid-like (focus, keyboard navigation, etc) functionality and now behaves more like a native, accessible table. See the Docs for new functionality.


I believe this should be more detailed and list out all the features that are no longer available in 5.0. However, it can be fleshed out in a future doc upgrade Jira and shouldn't prevent merging for now.

sdadn · 2023-10-11T15:48:31Z

packages/terra-table/src/Table.jsx

+  const [cellAriaLiveMessage, setCellAriaLiveMessage] = useState(null);
+
+  const [pinnedColumnOffsets, setPinnedColumnOffsets] = useState([0]);


shouldn't these be defined in utils/ColumnContext.jsx?

At some point I'd love to re-factor this and move the code around but like the original table columns logic from datagrid this would be non-trivial to change since AriaLiveMessage is used pretty much everywhere for the screen reader as is the pinned column offsets. We could probably log another JIRA to do this or flesh out the re-factor in another story.

Makes sense. That's fine then.

cm9361 · 2023-10-12T17:09:06Z

packages/terra-table/src/proptypes/cellShape.js

+  /**
+   *  Boolean indicating if cell contents are masked.
+   */
+  isMasked: PropTypes.bool,


The cell shape still does not have all of its props from the DataGrid package implementation.

cm9361 · 2023-10-12T17:10:06Z

packages/terra-table/src/proptypes/rowShape.js

+   * A string identifier used to describe the row contents. This value will be used to construct additional labels
+   * for internal controls (e.g. row selection cells).
+   */
+  ariaLabel: PropTypes.string,


The rowShape object does not have all of its props from the DataGrid implementation.

cm9361 · 2023-10-12T17:33:08Z

packages/terra-table/src/subcomponents/ColumnHeader.jsx

+            maximumWidth={column.maximumWidth}
+            headerHeight={headerHeight}
+            isResizable={column.isResizable}
+            isSelectable={isGridContext ? column.isSelectable !== false : column.isSelectable}


The check for column.isSelectable != false is not a present on the current ColumnHeader. With this logic, you are really adding a WorklistDataGrid specific requirement to the Table component. We should probably remove these checks and have WDG pass the appropriate value as Flowsheet does.

cm9361 · 2023-10-12T17:46:41Z

packages/terra-table/src/subcomponents/Row.jsx

+          key={`${id}_${displayedColumns[cellColumnIndex].id}`}
+          isSelected={!hasRowSelection && cellData.isSelected}
+          isMasked={cellData.isMasked}
+          isSelectable={isGridContext ? cellData.isSelectable !== false : false}


You don't need a ternary operator here. You could just check
isGridContext && cellData.isSelectable !== false

cm9361 · 2023-10-12T17:49:51Z

packages/terra-table/src/utils/_elementPolyfill.js

@@ -0,0 +1,19 @@
+// Source: https://www.npmjs.com/package/element-closest-polyfill


I cannot find where this polyfill file is being used? Maybe this file can be removed.

…aria live region to grid context.

cm9361 · 2023-10-13T16:18:10Z

packages/terra-table/src/utils/GridContext.jsx

+
+const GridContext = React.createContext({
+  role: 'table',
+  cellAriaLiveMessage: '',


This property should not be required. Nobody is reading the value. The table just needs the ability to call the callback.

cm9361 · 2023-10-13T16:28:42Z

packages/terra-table/src/subcomponents/Cell.jsx

+  return (
+    <CellTag
+      ref={cellRef}
+      aria-selected={isGridContext ? isSelected : undefined}


We should not have the isGridContext check here. We can just set it based on the selection state.

There is an edge case where aria-selected={false} in tables creates an accessibility violation, but since since we're saying that cell selection is a grid functionality (which setting the value is allowed) then we can just honor the value from the cell data. Done.

cm9361 · 2023-10-13T16:31:44Z

packages/terra-table/src/subcomponents/ColumnResizeHandle.jsx

+      draggable
+      role="slider"
+      tabIndex={isGridContext ? -1 : 0}
+      aria-hidden={isGridContext ? !isActive : undefined}


Does this really depend on the grid context?

I was facing an issue earlier where I was getting accessibility errors, but this works now. Done.

cm9361 · 2023-10-13T16:35:12Z

packages/terra-table/src/subcomponents/ColumnHeaderCell.jsx

+  const theme = useContext(ThemeContext);
+  const columnHeaderCell = useRef();
+
+  const isGridContext = gridContext.role === GridConstants.GRID;


Are we able to just made this a function in the GridContext that can be called instead of having to duplicate the logic in multiple files?

So it turns out that React contexts are read-only and we can't reference the values of a context inside itself. So This will have to do for now.

mjpalazzo · 2023-10-31T14:48:21Z

@kenk2 - Great work on Table and the examples! I have a few observations:
Keyboard navigation - there are no keyboard requirements for the Table pattern. I was able to tab to the table. For examples with scroll bars I was able use the down arrow keys to scroll through the table content. This provides sighted users with access to off screen content, so that is a valid behavior.
The Pinned Column table navigation with the Down Arrow key worked well. When I Down Arrowed to the right most cell and then pressed Down again, the cells hiddenb behind the pinned columns were visible again. However, when using the Up Arrow, I could not get the columns hidden behind the pinned columns completely visible.
JAWS Screen reader - From the Default Table header I used down arrow to go to the table. JAWS announced "Table with 10 columns and 3 rows." I used Down arrow to navigate the cells. For the table header cells, JAWS announced the role "button" even though there was no sorting enabled. For example, "Patient button." I think JAWS should announce "column header" @eawww - what are your thoughts on the header cell announcements?
Once I was in the body of the table, JAWS announced the cell content correctly. When I pressed Down Arrow from the last cell, JAWS announced, "Table end"
For the Sorted Table example:
APG recommends the following:
"To help screen reader users understand the purpose of the buttons in the column headers, an off-screen description of the sort functionality of the buttons is appended to the caption text. The description is added to the caption instead of to each button to prevent repetitious verbosity that could interfere with understanding of the column titles." I suggest that we add this suggested description to our example.
when I pressed Down Arrow to navigate to the first header cell, JAWS announced "Patient column header sorted ascending column 1" I pressed Enter which changed the sort order. JAWS did not announce the sort order. I tabbed to the column 2 header. JAWS announced "Location column header, column 2" The sort icon did not display (it should) and JAWS did not announce that the column is sortable or the sort order. JAWS should announce that the cell is interactable and how to interact with it. I pressed Enter and the sort order changed, but JAWS only announced "Enter" (Note that on the APG Sortable Table example page, JAWS did not announce the change in sort order either.)
That's all for now. I want to get these observations posted. Let me know if you would like to discuss.
I will continue testing the examples with focusable elements and also test with VoiceOver

kenk2 added 14 commits September 17, 2023 19:28

import datagrid code to table. Add dependencies from WDG package to t…

efe5480

…erra-table.

Merge remote-tracking branch 'origin' into new-terra-table

90e5187

Remove data-grid references in css class names. Fix some import paths.

716b903

Remove references to select and highlighting callbacks.

890c829

Delete old files.

3644f96

Remove old Table docs and tests.

ae6c3e3

Incorporate changes from deleted old table code.

a70ec07

Move folders out of un-necessary folder.

23b7e36

Update import paths.

0eb2c1c

Add translations. Remove references to selection css.

e6340b3

Remove hover style references.

ac486f8

Move translations to the correct path. Fix issue with pinned columns …

4420a3a

…background color.

Remove obsolete translations. Renamed translation strings. Delete som…

1427431

…e obsolete properties.

Get rid of some grid name references. Add a new example for pinned co…

8ac6f6b

…lumns.

kenk2 added the 📦 terra-table label Sep 20, 2023

kenk2 self-assigned this Sep 20, 2023

kenk2 added 6 commits September 20, 2023 07:42

Add some basic tests. Remove old imports and props.

a2dfdbd

Fix merge conflicts.

cbf259d

Remove obsolete prop. Fix some build errors.

cbf4027

Add and update tests for table component. Add back selectable props t…

4c20856

…o table for enabling column selections and callbacks. Add more examples for table.

Remove references to grid.

164a458

Add initial files for test demos.

9e3ae9c

sdadn reviewed Sep 22, 2023

View reviewed changes

packages/terra-core-docs/package.json Outdated Show resolved Hide resolved

kenk2 added 7 commits September 24, 2023 21:52

Added mock data files for wdio tests. Add basic suite of WDIO tests. …

701cf57

…Fix some behavior with cells focusing after navigation.

Add WDIO snapshots.

34a9825

Removed tabindex from cells and rows. Add functionality to make sorta…

7d4615f

…ble tables more interactable. Removed un-necessary ref initialization. Remove table container to handle scroll from consumers. Light refactor of pinned columns logic.

Update tests, add tabindex -1 to columnresizehandle for accessibility.

64f0c39

Update existing tests, add more tests for no interaction and sorting,…

72012ee

… focusing by keyboard.

Delete Terra outline view table project. Update changelog note.

102e027

Add upgrade guide for terra table.

50b59ff

github-actions bot temporarily deployed to preview-pr-3917 October 11, 2023 14:46 Destroyed

kenk2 requested review from cm9361 and sdadn October 11, 2023 14:49

smason0 reviewed Oct 11, 2023

View reviewed changes

Default rows to empty array. Update test example to reflect change.

dab96db

github-actions bot temporarily deployed to preview-pr-3917 October 11, 2023 15:21 Destroyed

sdadn reviewed Oct 11, 2023

View reviewed changes

sdadn approved these changes Oct 11, 2023

View reviewed changes

Default all columns to resizable if in grid context.

240a0a7

cm9361 reviewed Oct 12, 2023

View reviewed changes

kenk2 added 2 commits October 13, 2023 08:44

Make default cells unselectable and unresizeable. Update tests. Move …

be1c9a1

…aria live region to grid context.

Update snapshots of grid context.

da135da

github-actions bot temporarily deployed to preview-pr-3917 October 13, 2023 16:13 Destroyed

cm9361 reviewed Oct 13, 2023

View reviewed changes

kenk2 requested a review from cm9361 October 13, 2023 16:19

cm9361 reviewed Oct 13, 2023

View reviewed changes

github-actions bot temporarily deployed to preview-pr-3917 October 13, 2023 16:38 Destroyed

Do not check grid context in aria properties. Update tests.

41dcc44

github-actions bot temporarily deployed to preview-pr-3917 October 13, 2023 18:03 Destroyed

cm9361 approved these changes Oct 13, 2023

View reviewed changes

kenk2 merged commit bbfc1ca into table-mvb Oct 13, 2023
21 checks passed

kenk2 deleted the new-terra-table branch October 13, 2023 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-table] New Terra table #3917

[terra-table] New Terra table #3917

kenk2 commented Sep 20, 2023 •

edited

Loading

smason0 Oct 11, 2023

kenk2 Oct 11, 2023

smason0 Oct 11, 2023

kenk2 Oct 11, 2023 •

edited

Loading

smason0 Oct 11, 2023

kenk2 Oct 11, 2023

smason0 Oct 11, 2023

sdadn Oct 11, 2023

sdadn Oct 11, 2023

sdadn Oct 11, 2023

kenk2 Oct 11, 2023 •

edited

Loading

sdadn Oct 11, 2023

cm9361 Oct 12, 2023

cm9361 Oct 12, 2023

cm9361 Oct 12, 2023 •

edited

Loading

cm9361 Oct 12, 2023

cm9361 Oct 12, 2023 •

edited

Loading

cm9361 Oct 13, 2023

kenk2 Oct 13, 2023

cm9361 Oct 13, 2023

kenk2 Oct 13, 2023

cm9361 Oct 13, 2023

kenk2 Oct 13, 2023

cm9361 Oct 13, 2023

kenk2 Oct 13, 2023

mjpalazzo commented Oct 31, 2023


		## Changes from 4.x to 5.0

		Terra table will no longer support grid-like (focus, keyboard navigation, etc) functionality and now behaves more like a native, accessible table. See the Docs for new functionality.

		const [cellAriaLiveMessage, setCellAriaLiveMessage] = useState(null);

		const [pinnedColumnOffsets, setPinnedColumnOffsets] = useState([0]);

		@@ -0,0 +1,19 @@
		// Source: https://www.npmjs.com/package/element-closest-polyfill

[terra-table] New Terra table #3917

[terra-table] New Terra table #3917

Conversation

kenk2 commented Sep 20, 2023 • edited Loading

Summary

Testing

Reviews

Additional Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenk2 Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenk2 Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cm9361 Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cm9361 Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjpalazzo commented Oct 31, 2023

kenk2 commented Sep 20, 2023 •

edited

Loading

kenk2 Oct 11, 2023 •

edited

Loading

kenk2 Oct 11, 2023 •

edited

Loading

cm9361 Oct 12, 2023 •

edited

Loading

cm9361 Oct 12, 2023 •

edited

Loading