feat(Table): Adding table navigation by kolaps33 · Pull Request #2147 · microsoft/fluent-ui-react

kolaps33 · 2019-12-04T20:56:29Z

Adding table navigation like:

row by row navigation
navigate inside the row use right arrow key
in the row, then right and left arrow keys navigate between the cells

What should works:

when cell has more actionable elements, then:
- "Enter" key goes inside the cell
- "Escape" key goes outside the cell
- navigation between actionable elements inside cell is up/down and left/right arrows
when cell has one actionable element, then focus goes directly to the actionable element
space/enter invoke onclick on row
space/enter invoke onclick on actionable element(more option button)
click invoke onclick on whole row
click invoke onclick on actionable element(more option button)

Microsoft Reviewers: Open in CodeFlow

…ehavior

jurokapsiar · 2019-12-05T08:25:28Z

docs/src/examples/components/Table/Usage/TableExampleNavigable.shorthand.tsx

+      moreOptionButton,
+    ],
+    onClick: () => handleRowClick(1),
+    'aria-label': 'custom text',


what does custom text mean? I think it would be better to use meaningful labels in the examples

packages/react/src/components/Table/TableRow.tsx

jurokapsiar · 2019-12-05T08:31:43Z

can you please add handlesAccessibility tests to the unit tests of the table, row and cell?

silviuaavram · 2019-12-05T08:41:38Z

packages/accessibility/src/behaviors/Table/gridHeaderCellBehavior.ts

@@ -0,0 +1,22 @@
+import { Accessibility } from '../../types'
+import { IS_FOCUSABLE_ATTRIBUTE } from '../../attributes'
+// import * as keyboardKey from 'keyboard-key'


remove these comments

silviuaavram · 2019-12-05T08:41:53Z

packages/accessibility/src/behaviors/Table/gridHeaderRowBehavior.ts

+import * as keyboardKey from 'keyboard-key'
+import gridHeaderCellBehavior from './gridHeaderCellBehavior'
+
+// add key actions unit tests


silviuaavram · 2019-12-05T08:47:36Z

packages/accessibility/src/behaviors/Table/gridHeaderRowBehavior.ts

+// add key actions unit tests
+
+/**
+ * @description


if you don't add anything to description just remove them, otherwise add description about the behavior.

packages/accessibility/src/behaviors/Table/gridNestedBehavior.ts

silviuaavram · 2019-12-05T08:53:11Z

packages/accessibility/test/behaviors/testDefinitions.ts

  },
 })

+// Triggers 'unsetRowTabbable' action using SHIFT + TAB key on 'root'.


Maybe we should abstract this, so we can include stuff like Triggers 'openAndFocusFirst' action with 'altKey'+'ArrowDown' on 'root' slot.

Maybe we can also add the part with based on the prop $propname so we ca include stuff like:

...(!props.open && { openAndFocusFirst: { keyCombinations: [{ keyCode: keyboardKey.ArrowDown, altKey: true }], }, }),

silviuaavram · 2019-12-05T08:55:07Z

packages/react-bindings/src/FocusZone/FocusZone.tsx


    const doc = getDocument(this._root.current)

+    if (this.props.onKeyDown) {


you can probably use lodash invoke here: _.invoke(this.props, 'onKeyDown', ev)

as this is FocusZone code I rather don't want to change the code...
Even this change we are asking if focusZone people agree with the change.

jurokapsiar · 2019-12-06T09:19:07Z

docs/src/examples/components/Table/BestPractices/TableBestPractices.tsx

+    Choose desired accessibility behavior for grid cell depending on the use case.(Check the{' '}
+    {link('Behaviors', '/components/table/accessibility')} section).
+  </Text>,
+  'Provide label to the Table component using `aria-label` or `aria-labelledby` prop.',


pls add: If no label is provided, the whole header will be read by the screen reader when the user navigates to the table

jurokapsiar · 2019-12-06T09:21:13Z

docs/src/examples/components/Table/BestPractices/TableBestPractices.tsx

+    {link('Behaviors', '/components/table/accessibility')} section).
+  </Text>,
+  'Provide label to the Table component using `aria-label` or `aria-labelledby` prop.',
+  'Provide label to the Row component using `aria-label` or `aria-labelledby` prop. If not, then each cell of the row is narrated by screen reader.',


pls change to Consider providing label to the TableRow component using aria-label or aria-labelledby prop. When not provided, the whole row (all cells) will ne narrated by the screen reader when the user navigates to the row.

jurokapsiar · 2019-12-06T09:26:48Z

docs/src/examples/components/Table/BestPractices/TableBestPractices.tsx

+  'Provide label to the Table component using `aria-label` or `aria-labelledby` prop.',
+  'Provide label to the Row component using `aria-label` or `aria-labelledby` prop. If not, then each cell of the row is narrated by screen reader.',
+  'Provide label to the table header column, if cell has no content.',
+  'Stop event propagation, when you will add actionable element into the grid cell.',


If you use actionable element or component in the cell, stop propagation of the click event if you do not want the interacion with the element to perform row click action.

jurokapsiar · 2019-12-06T09:57:10Z

docs/src/examples/components/Table/BestPractices/TableBestPractices.tsx

+]
+
+const dontList = [
+  "Don't set onClick action on row, which will be not available in the cell of grid, as well. With screen reader navigation user can navigate only cell by cell.",


If you use onClick on the row, make also one (preferably the first) cell actionable and add onClick handler with the same action to it. Screen readers do not navigate to rows, so actions need to be available on cells as well.

jurokapsiar · 2019-12-12T13:59:24Z

docs/src/examples/components/Table/Usage/TableExampleNavigable.shorthand.tsx

+
+function tagButton(tagName: string) {
+  return (
+    <Provider


make sure to have only one provider in the example, not for each button

…t-ui/react into mituron/table-navigation

mituron added 4 commits November 28, 2019 19:25

initial commit for table navigation

86813f7

another multiple changes

dad5ae4

adding behavior specification, test, adding childBehaviors logic to b…

719a36f

…ehavior

removing leftovers

cd5af10

kolaps33 requested review from JasonGore, dvdzkwsk, dzearing, ecraig12345, jdhuntington, kenotron, layershifter, levithomason, miroslavstastny and mnajdova as code owners December 4, 2019 20:56

kolaps33 changed the title ~~Adding table navigation~~ feat(Table): Adding table navigation Dec 4, 2019

jurokapsiar reviewed Dec 5, 2019

View reviewed changes

packages/react/src/components/Table/TableRow.tsx Show resolved Hide resolved

silviuaavram reviewed Dec 5, 2019

View reviewed changes

kolaps33 mentioned this pull request Dec 5, 2019

Refactor ability to shift+TAB from table cell #2150

Open

mituron added 3 commits December 5, 2019 18:33

adressing comments from PR

760c51c

add documentation

112bd12

adding documentation, best practices, bug

7dac8df

jurokapsiar reviewed Dec 6, 2019

View reviewed changes

improving the doc

6a5715e

kolaps33 mentioned this pull request Dec 6, 2019

fix(Accessibility): Always handle provided onKeyDown event be propagated in inner zone #2140

Merged

reverting changeds in focusZone

3cd668f

mituron and others added 18 commits December 6, 2019 17:00

reverting back changes in fz

2809b57

again trying to revert changes

9b04145

experiment

a9b22ec

adding back fz in hope to remove format trouble

2cf8c45

Merge branch 'master' into mituron/table-navigation

5b3075b

removing focuszone some experiment

45f704e

adding FZ from master

9837625

improving column header name, adding one more JAWS bug

0db3361

Merge branch 'master' into mituron/table-navigation

c7dbe1e

experiment with index

572d56f

fix of the unit tests and code

851653c

change table behaviors logic

ae3f4df

adding screener

ea3c102

adding accessibility into rederConfig

e37b2e3

Merge branch 'master' into mituron/table-navigation

42ae406

removing wrong import

a376d49

merge conflict solving

6f97e06

adding back import

3cdc0ba

jurokapsiar reviewed Dec 12, 2019

View reviewed changes

remove styles from example

49d0e29

jurokapsiar approved these changes Dec 12, 2019

View reviewed changes

mituron and others added 3 commits December 12, 2019 15:21

amending changelog

0a255a2

Merge branch 'mituron/table-navigation' of https://github.com/stardus…

334a664

…t-ui/react into mituron/table-navigation

Merge branch 'master' into mituron/table-navigation

ed28221

kolaps33 merged commit f82c8a0 into master Dec 12, 2019

kolaps33 deleted the mituron/table-navigation branch December 12, 2019 15:55


		const doc = getDocument(this._root.current)

		if (this.props.onKeyDown) {

Conversation

kolaps33 commented Dec 4, 2019 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jurokapsiar commented Dec 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kolaps33 commented Dec 4, 2019 •

edited by ghost

Loading