From 9a52d5321c60e6e340aa7e0e85fd43085cc219ba Mon Sep 17 00:00:00 2001 From: Eugene Olonov Date: Tue, 1 Mar 2022 11:16:55 -0800 Subject: [PATCH 1/3] a11y: fix table-forms keyboard navigation The PR aims to address the following issues found on pages with lists containing nested forms, including: "Bot Responses", "Knowledge (QnA)" and "Knowledge Base" pages. Here is the list of issues addressed: - Unable to navigate to fields nested in DetailsList cells using keyboard - Some buttons are hidden while navigating via keyboard ("Clear field" button, "Add new" item button) - Save via keyboard Enter key doesn't work under several circumstances - Shift+Enter should persist caret position and the field in focus while adding a new line - Visual focus indication for focused fields works unstable - No error announcement - No way to blur selected fields when the focus is on the DetailsList cell field Highlights on changes made: - Implement a CellFocusZone component to handle focus in DetailsLists' cells having form elements - Move all related logic to the component, so it can: focus form elements, handle Escape and Enter key and handle focus loss cases - Update EditField to play well with the new component - Add a callback to be called on EditField when a new line gets added - Handle Shift+Enter properly: from now it adds a new row to input and resets focus back to the input after it got switched to the textarea element - Update CSS rules to handle focus for tables-view and EditableField components - Refactor styles: move shared styles to the CellFocusZone - Refactor table-views: introduce CellFocusZone and improve keyboard handling - Knowledge Base: change the way alternative questions added - Fix EditField styles to handle field height correctly - Add announcements for EdtiField errors --- .../client/src/components/CellFocusZone.tsx | 100 ++++++++++++++++++ .../client/src/components/EditableField.tsx | 31 +++++- .../client/src/pages/knowledge-base/styles.ts | 2 +- .../src/pages/knowledge-base/table-view.tsx | 22 ++-- .../pages/language-generation/table-view.tsx | 21 ++-- .../pages/language-understanding/styles.ts | 10 -- .../language-understanding/table-view.tsx | 23 ++-- 7 files changed, 165 insertions(+), 44 deletions(-) create mode 100644 Composer/packages/client/src/components/CellFocusZone.tsx diff --git a/Composer/packages/client/src/components/CellFocusZone.tsx b/Composer/packages/client/src/components/CellFocusZone.tsx new file mode 100644 index 0000000000..69441ff7aa --- /dev/null +++ b/Composer/packages/client/src/components/CellFocusZone.tsx @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +/** @jsx jsx */ +import { jsx, css } from '@emotion/core'; +import React, { useCallback } from 'react'; +import { FocusZone, FocusZoneTabbableElements, IFocusZoneProps } from 'office-ui-fabric-react/lib/FocusZone'; +import { getFocusStyle, getTheme, mergeStyles } from 'office-ui-fabric-react/lib/Styling'; + +export const formCell = css` + outline: none; + white-space: pre-wrap; + font-size: 14px; + line-height: 28px; +`; + +export const formCellFocus = mergeStyles( + getFocusStyle(getTheme(), { + inset: -3, + }) +); + +/** + * CellFocusZone component props. + * Props are intently omitted as they're required for component to function correctly. + * Please manually test the component using keyboard in places it is used in case any changes made. + */ +type CellFocusZoneProps = Omit< + React.HTMLAttributes & IFocusZoneProps, + | 'allowFocusRoot' + | 'data-is-focusable' + | 'isCircularNavigation' + | 'className' + | 'handleTabKey' + | 'shouldFocusInnerElementWhenReceivedFocus' + | 'tabIndex' + | 'onBlur' + | 'onKeyDown' +>; + +const CellFocusZone: React.FC = (props) => { + const focusCurrentZoneAfterRender = useCallback((focusZoneEl: any) => { + const focusZoneId = focusZoneEl?.dataset?.focuszoneId; + if (!focusZoneId) { + return; + } + // wait for render to happen before placing focus back to the focus zone + setTimeout(() => { + const focusZone: HTMLElement | null = document.querySelector(`[data-focuszone-id=${focusZoneId}]`); + focusZone?.focus(); + }, 100); + }, []); + + const onCellKeyDown = useCallback((ev) => { + // ignore any of events coming from input fields + if (ev.target.localName === 'input' || ev.target.localName === 'textarea') { + ev.stopPropagation(); + return; + } + + // enter inside cell using Enter key + if (ev.target.dataset.focuszoneId && ev.key === 'Enter') { + const input: HTMLElement | null = (ev?.target as HTMLElement)?.querySelector('a, button, input, textarea'); + input?.focus(); + return; + } + + // handle uncaught escape key events to allow returning back to navigation between cells + if (ev.key === 'Escape') { + ev.stopPropagation(); + focusCurrentZoneAfterRender(ev.currentTarget); + } + }, []); + + const onCellInnerBlur = useCallback((ev) => { + // this means we dont have an element to focus so we place focus back to the cell + if (!ev.relatedTarget) { + focusCurrentZoneAfterRender(ev.currentTarget); + } + }, []); + + return ( + + {props.children} + + ); +}; + +export { CellFocusZone }; diff --git a/Composer/packages/client/src/components/EditableField.tsx b/Composer/packages/client/src/components/EditableField.tsx index 8bc59b6897..e2d1703423 100644 --- a/Composer/packages/client/src/components/EditableField.tsx +++ b/Composer/packages/client/src/components/EditableField.tsx @@ -8,6 +8,7 @@ import { NeutralColors, SharedColors } from '@uifabric/fluent-theme'; import { mergeStyleSets } from '@uifabric/styling'; import { IconButton } from 'office-ui-fabric-react/lib/Button'; import { IIconProps } from 'office-ui-fabric-react/lib/Icon'; +import { Announced } from 'office-ui-fabric-react/lib/Announced'; import { FieldConfig, useForm } from '../hooks/useForm'; @@ -24,10 +25,12 @@ const defaultContainerStyle = (hasFocus, hasErrors) => css` : undefined}; background: ${hasFocus || hasErrors ? NeutralColors.white : 'inherit'}; margin-top: 2px; - :hover .ms-Button-icon { + :hover .ms-Button-icon, + :focus-within .ms-Button-icon { visibility: visible; } .ms-TextField-field { + min-height: 35px; cursor: pointer; padding-left: ${hasFocus || hasErrors ? '8px' : '0px'}; :focus { @@ -64,7 +67,6 @@ interface EditableFieldProps extends Omit; transparentBorder?: boolean; ariaLabel?: string; - error?: string | JSX.Element; extraContent?: string; containerStyles?: SerializedStyles; className?: string; @@ -81,9 +83,11 @@ interface EditableFieldProps extends Omit void; onChange: (newValue?: string) => void; onFocus?: () => void; + onNewLine?: () => void; } const EditableField: React.FC = (props) => { @@ -102,6 +106,7 @@ const EditableField: React.FC = (props) => { onChange, onFocus, onBlur, + onNewLine, value, id, error, @@ -192,13 +197,29 @@ const EditableField: React.FC = (props) => { e.stopPropagation(); } const enterOnField = e.key === 'Enter' && hasFocus; - if (enterOnField && !multiline) { + if (enterOnField && !multiline && e.shiftKey) { + e.stopPropagation(); + if (onNewLine) { + onNewLine(); + return; + } setMultiline(true); + updateField('value', e.target.value + '\n'); + // wait for the textarea to be rendered and then restore focus and selection + setTimeout(() => { + const len = fieldRef.current?.value?.length; + fieldRef.current?.focus(); + if (len) { + fieldRef.current?.setSelectionRange(len, len); + } + }, 100); } if (enterOnField && !e.shiftKey) { - handleCommit(); + // blur triggers commit, so call blur to avoid multiple commits + fieldRef.current?.blur(); } if (e.key === 'Escape') { + e.stopPropagation(); cancel(); } }; @@ -298,6 +319,8 @@ const EditableField: React.FC = (props) => { {requiredMessage || formErrors.value} )} {error && {error}} + {hasErrors && hasBeenEdited && } + {error && } ); }; diff --git a/Composer/packages/client/src/pages/knowledge-base/styles.ts b/Composer/packages/client/src/pages/knowledge-base/styles.ts index e18a385447..e5d5e272b8 100644 --- a/Composer/packages/client/src/pages/knowledge-base/styles.ts +++ b/Composer/packages/client/src/pages/knowledge-base/styles.ts @@ -94,7 +94,7 @@ export const rowDetails = { '.ms-GroupHeader-expand': { fontSize: 8, }, - '&:hover': { + '&:hover, &:focus-within': { background: NeutralColors.gray30, selectors: { '.ms-TextField-fieldGroup': { diff --git a/Composer/packages/client/src/pages/knowledge-base/table-view.tsx b/Composer/packages/client/src/pages/knowledge-base/table-view.tsx index dd0a14375a..ff1757d49c 100644 --- a/Composer/packages/client/src/pages/knowledge-base/table-view.tsx +++ b/Composer/packages/client/src/pages/knowledge-base/table-view.tsx @@ -15,6 +15,7 @@ import { IDetailsGroupRenderProps, IGroup, IDetailsList, + IColumn, } from 'office-ui-fabric-react/lib/DetailsList'; import { GroupHeader, CollapseAllVisibility } from 'office-ui-fabric-react/lib/GroupedList'; import { IOverflowSetItemProps, OverflowSet } from 'office-ui-fabric-react/lib/OverflowSet'; @@ -43,6 +44,7 @@ import { EditQnAModal } from '../../components/QnA/EditQnAFrom'; import { ReplaceQnAFromModal } from '../../components/QnA/ReplaceQnAFromModal'; import { getQnAFileUrlOption } from '../../utils/qnaUtil'; import TelemetryClient from '../../telemetry/TelemetryClient'; +import { CellFocusZone } from '../../components/CellFocusZone'; import { formCell, @@ -523,7 +525,7 @@ const TableView: React.FC = (props) => { ); const getTableColums = () => { - const tableColums = [ + const tableColums: IColumn[] = [ { key: 'ToggleShowAll', name: '', @@ -566,7 +568,8 @@ const TableView: React.FC = (props) => { const addQuestionButton = ( { + onClick={(ev) => { + ev.stopPropagation(); setCreatingQuestionInKthSection(item.sectionId); TelemetryClient.track('AlternateQnAPhraseAdded'); }} @@ -576,7 +579,7 @@ const TableView: React.FC = (props) => { ); return ( -
+ {questions.map((question, qIndex: number) => { const isQuestionEmpty = question.content === ''; const isOnlyQuestion = questions.length === 1 && qIndex === 0; @@ -627,6 +630,9 @@ const TableView: React.FC = (props) => { }} onChange={() => {}} onFocus={() => setExpandedIndex(index)} + onNewLine={() => { + setCreatingQuestionInKthSection(item.sectionId); + }} />
); @@ -674,11 +680,12 @@ const TableView: React.FC = (props) => { }} onChange={() => {}} onFocus={() => setExpandedIndex(index)} + onNewLine={() => {}} /> ) : ( addQuestionButton )} - + ); }, }, @@ -698,7 +705,7 @@ const TableView: React.FC = (props) => { item.fileId === createQnAPairSettings.groupKey && index === createQnAPairSettings.sectionIndex; return ( -
+ = (props) => { onChange={() => {}} onFocus={() => setExpandedIndex(index)} /> -
+ ); }, }, @@ -755,7 +762,7 @@ const TableView: React.FC = (props) => { data: 'string', onRender: (item) => { return ( -
+
{item.usedIn.map(({ id, displayName }) => { return ( = (props) => { checkboxVisibility={CheckboxVisibility.hidden} columns={getTableColums()} componentRef={detailListRef} + getKey={(item, index) => `row-${index}`} groupProps={{ onRenderHeader: onRenderGroupHeader, collapseAllVisibility: CollapseAllVisibility.hidden, diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index 2b98abeae3..be13c416c5 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -22,11 +22,12 @@ import { LgFile } from '@botframework-composer/types/src'; import { EditableField } from '../../components/EditableField'; import { navigateTo } from '../../utils/navigation'; -import { actionButton, formCell, editableFieldContainer } from '../language-understanding/styles'; +import { actionButton, editableFieldContainer } from '../language-understanding/styles'; import { dispatcherState, localeState, settingsState, dialogsSelectorFamily } from '../../recoilModel'; import { languageListTemplates } from '../../components/MultiLanguage'; import TelemetryClient from '../../telemetry/TelemetryClient'; import { lgFilesSelectorFamily } from '../../recoilModel/selectors/lg'; +import { CellFocusZone } from '../../components/CellFocusZone'; interface TableViewProps extends RouteComponentProps<{ dialogId: string; skillId: string; projectId: string }> { projectId: string; @@ -60,8 +61,6 @@ const TableView: React.FC = (props) => { const activeDialog = dialogs.find(({ id }) => id === dialogId); - //const [focusedIndex, setFocusedIndex] = useState(0); - useEffect(() => { if (!file || isEmpty(file)) return; @@ -207,7 +206,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const displayName = `#${item.name}`; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -238,7 +237,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item.body; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -274,7 +273,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item.body; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -309,7 +308,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item[`body-${defaultLanguage}`]; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, diff --git a/Composer/packages/client/src/pages/language-understanding/styles.ts b/Composer/packages/client/src/pages/language-understanding/styles.ts index 879c07e14c..929edee5e6 100644 --- a/Composer/packages/client/src/pages/language-understanding/styles.ts +++ b/Composer/packages/client/src/pages/language-understanding/styles.ts @@ -23,16 +23,6 @@ export const codeEditorContainer = css` width: 100%; `; -export const formCell = css` - outline: none; - :focus { - outline: rgb(102, 102, 102) solid 1px; - } - white-space: pre-wrap; - font-size: 14px; - line-height: 28px; -`; - export const luPhraseCell = css` outline: none; :focus { diff --git a/Composer/packages/client/src/pages/language-understanding/table-view.tsx b/Composer/packages/client/src/pages/language-understanding/table-view.tsx index 8dd80df04e..d9df5dc85e 100644 --- a/Composer/packages/client/src/pages/language-understanding/table-view.tsx +++ b/Composer/packages/client/src/pages/language-understanding/table-view.tsx @@ -31,8 +31,10 @@ import { dialogsSelectorFamily, luFilesSelectorFamily, } from '../../recoilModel'; +import { CellFocusZone } from '../../components/CellFocusZone'; + +import { luPhraseCell, tableCell, editableFieldContainer } from './styles'; -import { formCell, luPhraseCell, tableCell, editableFieldContainer } from './styles'; interface TableViewProps extends RouteComponentProps<{ dialogId: string; skillId: string; projectId: string }> { projectId: string; skillId?: string; @@ -199,7 +201,7 @@ const TableView: React.FC = (props) => { onRender: (item: Intent) => { const displayName = `#${item.name}`; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -231,7 +233,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item.phrases; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -265,7 +267,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item.phrases; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -298,7 +300,7 @@ const TableView: React.FC = (props) => { onRender: (item) => { const text = item[`body-${defaultLanguage}`]; return ( -
+ = (props) => { }} onChange={() => {}} /> -
+ ); }, }, @@ -338,7 +340,6 @@ const TableView: React.FC = (props) => { return (
navigateTo(`${baseURL}dialogs/${id}`)} > @@ -399,7 +400,7 @@ const TableView: React.FC = (props) => { data: 'string', onRender: (item) => { return ( -
+
{item.state}
From dcbe7453f2ecd37109b4168048c99df4dd8d22de Mon Sep 17 00:00:00 2001 From: Eugene Olonov Date: Tue, 1 Mar 2022 12:39:26 -0800 Subject: [PATCH 2/3] Revert types changes --- .../packages/client/src/pages/knowledge-base/table-view.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Composer/packages/client/src/pages/knowledge-base/table-view.tsx b/Composer/packages/client/src/pages/knowledge-base/table-view.tsx index ff1757d49c..beb006440b 100644 --- a/Composer/packages/client/src/pages/knowledge-base/table-view.tsx +++ b/Composer/packages/client/src/pages/knowledge-base/table-view.tsx @@ -15,7 +15,6 @@ import { IDetailsGroupRenderProps, IGroup, IDetailsList, - IColumn, } from 'office-ui-fabric-react/lib/DetailsList'; import { GroupHeader, CollapseAllVisibility } from 'office-ui-fabric-react/lib/GroupedList'; import { IOverflowSetItemProps, OverflowSet } from 'office-ui-fabric-react/lib/OverflowSet'; @@ -525,7 +524,7 @@ const TableView: React.FC = (props) => { ); const getTableColums = () => { - const tableColums: IColumn[] = [ + const tableColums = [ { key: 'ToggleShowAll', name: '', From 6e349933f3d9a0607a292fa1125e5066851874e3 Mon Sep 17 00:00:00 2001 From: Eugene Olonov Date: Thu, 3 Mar 2022 12:11:52 -0800 Subject: [PATCH 3/3] Refactor timeouts --- .../client/src/components/CellFocusZone.tsx | 7 +++-- .../client/src/components/EditableField.tsx | 6 ++-- .../client/src/hooks/useAfterRender.ts | 29 +++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 Composer/packages/client/src/hooks/useAfterRender.ts diff --git a/Composer/packages/client/src/components/CellFocusZone.tsx b/Composer/packages/client/src/components/CellFocusZone.tsx index 69441ff7aa..24e769c387 100644 --- a/Composer/packages/client/src/components/CellFocusZone.tsx +++ b/Composer/packages/client/src/components/CellFocusZone.tsx @@ -6,6 +6,8 @@ import React, { useCallback } from 'react'; import { FocusZone, FocusZoneTabbableElements, IFocusZoneProps } from 'office-ui-fabric-react/lib/FocusZone'; import { getFocusStyle, getTheme, mergeStyles } from 'office-ui-fabric-react/lib/Styling'; +import { useAfterRender } from '../hooks/useAfterRender'; + export const formCell = css` outline: none; white-space: pre-wrap; @@ -38,16 +40,17 @@ type CellFocusZoneProps = Omit< >; const CellFocusZone: React.FC = (props) => { + const onAfterRender = useAfterRender(); const focusCurrentZoneAfterRender = useCallback((focusZoneEl: any) => { const focusZoneId = focusZoneEl?.dataset?.focuszoneId; if (!focusZoneId) { return; } // wait for render to happen before placing focus back to the focus zone - setTimeout(() => { + onAfterRender(() => { const focusZone: HTMLElement | null = document.querySelector(`[data-focuszone-id=${focusZoneId}]`); focusZone?.focus(); - }, 100); + }); }, []); const onCellKeyDown = useCallback((ev) => { diff --git a/Composer/packages/client/src/components/EditableField.tsx b/Composer/packages/client/src/components/EditableField.tsx index e2d1703423..3576715594 100644 --- a/Composer/packages/client/src/components/EditableField.tsx +++ b/Composer/packages/client/src/components/EditableField.tsx @@ -11,6 +11,7 @@ import { IIconProps } from 'office-ui-fabric-react/lib/Icon'; import { Announced } from 'office-ui-fabric-react/lib/Announced'; import { FieldConfig, useForm } from '../hooks/useForm'; +import { useAfterRender } from '../hooks/useAfterRender'; const allowedNavigationKeys = ['ArrowDown', 'ArrowUp', 'ArrowLeft', 'ArrowRight', 'PageDown', 'PageUp', 'Home', 'End']; @@ -119,6 +120,7 @@ const EditableField: React.FC = (props) => { const [hasFocus, setHasFocus] = useState(false); const [hasBeenEdited, setHasBeenEdited] = useState(false); const [multiline, setMultiline] = useState(true); + const onAfterRender = useAfterRender(); const formConfig: FieldConfig<{ value: string }> = { value: { @@ -206,13 +208,13 @@ const EditableField: React.FC = (props) => { setMultiline(true); updateField('value', e.target.value + '\n'); // wait for the textarea to be rendered and then restore focus and selection - setTimeout(() => { + onAfterRender(() => { const len = fieldRef.current?.value?.length; fieldRef.current?.focus(); if (len) { fieldRef.current?.setSelectionRange(len, len); } - }, 100); + }); } if (enterOnField && !e.shiftKey) { // blur triggers commit, so call blur to avoid multiple commits diff --git a/Composer/packages/client/src/hooks/useAfterRender.ts b/Composer/packages/client/src/hooks/useAfterRender.ts new file mode 100644 index 0000000000..6f79d9a6d4 --- /dev/null +++ b/Composer/packages/client/src/hooks/useAfterRender.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { useCallback, useLayoutEffect, useRef } from 'react'; + +/** + * Run a callback function not earlier than immediate re-renders happen + * @returns onAfterRender + */ +export const useAfterRender = () => { + const timeout = useRef(); + const callback = useRef<(() => void) | null>(null); + + useLayoutEffect(() => { + callback.current?.(); + }); + + return useCallback((fn: () => unknown) => { + callback.current = () => { + if (timeout.current) clearTimeout(timeout.current); + timeout.current = setTimeout(() => { + callback.current = null; + fn(); + }, 0); + }; + + callback.current?.(); + }, []); +};