From fa16ea15f125962088e14b7927ebb0512213b49e Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 15 Jun 2021 11:42:08 -0700 Subject: [PATCH 1/8] Add typings for Adaptive Cards --- packages/component/src/hooks/internal/useUniqueId.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/component/src/hooks/internal/useUniqueId.js b/packages/component/src/hooks/internal/useUniqueId.js index c9d73c5027..0c21f3d7fd 100644 --- a/packages/component/src/hooks/internal/useUniqueId.js +++ b/packages/component/src/hooks/internal/useUniqueId.js @@ -3,7 +3,7 @@ import { useMemo } from 'react'; import random from 'math-random'; -export default function useUniqueId(prefix) { +export default function useUniqueId(prefix?: string): string { const id = useMemo(() => random().toString(36).substr(2, 5), []); prefix = prefix ? `${prefix}--` : ''; From 4baef77caccdf72efcad295770acf80b482d081a Mon Sep 17 00:00:00 2001 From: William Wong Date: Wed, 16 Jun 2021 14:34:11 -0700 Subject: [PATCH 2/8] Move to .ts --- .../api/src/types/CardActionMiddleware.ts | 33 +++++++------------ ...rdRenderer.js => AdaptiveCardRenderer.tsx} | 8 ++--- .../bundle/src/types/AdaptiveCardsPackage.ts | 13 +++++++- 3 files changed, 28 insertions(+), 26 deletions(-) rename packages/bundle/src/adaptiveCards/Attachment/{AdaptiveCardRenderer.js => AdaptiveCardRenderer.tsx} (98%) diff --git a/packages/api/src/types/CardActionMiddleware.ts b/packages/api/src/types/CardActionMiddleware.ts index e0405783c8..30bc3de33f 100644 --- a/packages/api/src/types/CardActionMiddleware.ts +++ b/packages/api/src/types/CardActionMiddleware.ts @@ -2,28 +2,19 @@ import { DirectLineCardAction } from 'botframework-webchat-core'; import FunctionMiddleware, { CallFunction } from './FunctionMiddleware'; -type PerformCardAction = CallFunction< - [ - { - cardAction: DirectLineCardAction; - getSignInUrl?: () => string; - target: any; - } - ], - void ->; +type PerformCardActionParameter = { + cardAction?: DirectLineCardAction; + displayText?: string; + getSignInUrl?: () => string; + target?: any; + text?: string; + type?: string; + value?: any; +}; -type CardActionMiddleware = FunctionMiddleware< - [{ dispatch: (action: any) => void }], - [ - { - cardAction: DirectLineCardAction; - getSignInUrl?: () => string; - target: any; - } - ], - {} ->; +type PerformCardAction = CallFunction<[PerformCardActionParameter], void>; + +type CardActionMiddleware = FunctionMiddleware<[{ dispatch: (action: any) => void }], [PerformCardActionParameter], {}>; export default CardActionMiddleware; diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx similarity index 98% rename from packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js rename to packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx index ab3e2353d2..05e7e6253d 100644 --- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.js +++ b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx @@ -131,7 +131,7 @@ function disableElementWithUndo(element) { return () => undoStack.forEach(undo => undo && undo()); } -function disableInputElementsWithUndo(element, observeSubtree = true) { +function disableInputElementsWithUndo(element: HTMLElement, observeSubtree = true) { const undoStack = [].map.call(element.querySelectorAll('button, input, select, textarea'), element => disableElementWithUndo(element) ); @@ -145,7 +145,7 @@ function disableInputElementsWithUndo(element, observeSubtree = true) { if (observeSubtree) { const observer = new MutationObserver(mutations => mutations.forEach(({ addedNodes }) => - undoStack.push(...addedNodes.map(addedNode => disableInputElementsWithUndo(addedNode, false))) + undoStack.push(...[].map.call(addedNodes, addedNode => disableInputElementsWithUndo(addedNode, false))) ) ); @@ -232,8 +232,8 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled const [errors, setErrors] = useState([]); const [lastRender, setLastRender] = useState(0); const activeElementIndexRef = useRef(-1); - const adaptiveCardElementRef = useRef(); - const contentRef = useRef(); + const adaptiveCardElementRef = useRef(); + const contentRef = useRef(); const inputValuesRef = useRef([]); const localize = useLocalizer(); const performCardAction = usePerformCardAction(); diff --git a/packages/bundle/src/types/AdaptiveCardsPackage.ts b/packages/bundle/src/types/AdaptiveCardsPackage.ts index 52eddda3b7..4ca8c4d416 100644 --- a/packages/bundle/src/types/AdaptiveCardsPackage.ts +++ b/packages/bundle/src/types/AdaptiveCardsPackage.ts @@ -1,8 +1,19 @@ -import { AdaptiveCard, HorizontalAlignment, TextSize, TextWeight, SerializationContext, Version } from 'adaptivecards'; +import { + AdaptiveCard, + GlobalSettings, + HorizontalAlignment, + HostConfig, + TextSize, + TextWeight, + SerializationContext, + Version +} from 'adaptivecards'; type AdaptiveCardsPackage = { AdaptiveCard: typeof AdaptiveCard; + GlobalSettings: typeof GlobalSettings; HorizontalAlignment: typeof HorizontalAlignment; + HostConfig: typeof HostConfig; TextSize: typeof TextSize; TextWeight: typeof TextWeight; SerializationContext: typeof SerializationContext; From d1b0ab44dbff1d067b96ed5ac3d87900a6873378 Mon Sep 17 00:00:00 2001 From: William Wong Date: Wed, 16 Jun 2021 18:12:47 -0700 Subject: [PATCH 3/8] Remove role="menubar" after submission --- CHANGELOG.md | 1 + ...accessibility.adaptiveCard.ariaPushed.html | 206 ++++++++++++++++++ .../Attachment/AdaptiveCardRenderer.tsx | 196 +++++++++++++++-- 3 files changed, 379 insertions(+), 24 deletions(-) create mode 100644 __tests__/html/accessibility.adaptiveCard.ariaPushed.html diff --git a/CHANGELOG.md b/CHANGELOG.md index d72f4d6160..575626e603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixes [#3825](https://github.com/microsoft/BotFramework-WebChat/issues/3825). Add `role="presentation"` to all decorative ``, by [@compulim](https://github.com/compulim), in PR [#3903](https://github.com/microsoft/BotFramework-WebChat/pull/3903) - Fixes [#3360](https://github.com/microsoft/BotFramework-WebChat/issues/3360) and [#3615](https://github.com/microsoft/BotFramework-WebChat/issues/3615). Use `channelData['webchat:fallback-text']` field for screen reader text, before stripping Markdown from [`activity.text` field](https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#text), by [@compulim](https://github.com/compulim), in PR [#3917](https://github.com/microsoft/BotFramework-WebChat/pull/3917) - Fixes [#3856](https://github.com/microsoft/BotFramework-WebChat/issues/3856). Fix missing typings, by [@compulim](https://github.com/compulim) and [@corinagum](https://github.com/corinagum), in PR [#3931](https://github.com/microsoft/BotFramework-WebChat/pull/3931) +- Fixes [#3947](https://github.com/microsoft/BotFramework-WebChat/issues/3947). Adaptive Cards: all action sets (which has `role="menubar"`) must have at least 1 or more `role="menuitem"`, by [@compulim](https://github.com/compulim), in PR [#XXX](https://github.com/microsoft/BotFramework-WebChat/pull/XXX) ### Changed diff --git a/__tests__/html/accessibility.adaptiveCard.ariaPushed.html b/__tests__/html/accessibility.adaptiveCard.ariaPushed.html new file mode 100644 index 0000000000..da7337b4f7 --- /dev/null +++ b/__tests__/html/accessibility.adaptiveCard.ariaPushed.html @@ -0,0 +1,206 @@ + + + + + + + + + +
+ + + diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx index 05e7e6253d..84b18e6c26 100644 --- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx +++ b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx @@ -157,6 +157,167 @@ function disableInputElementsWithUndo(element: HTMLElement, observeSubtree = tru return () => undoStack.forEach(undo => undo && undo()); } +/** + * Checks if an element contains a class. + * + * @param {HTMLElement} element - The element to check for the class. + * @param {string} className - The name of the class to check for. + * @returns {boolean} `true` if the element contains the class, otherwise, `false`. + */ +function containsClassName(element: HTMLElement, className: string): boolean { + return (element.className || '').split(' ').includes(className); +} + +/** + * Gets the value of an attribute from an element. + * + * @returns {false | string} The value of the attribute. `false` if the attribute was not set. + */ +function getAttribute(element: HTMLElement, qualifiedName: string): false | string { + return element.hasAttribute(qualifiedName) && element.getAttribute(qualifiedName); +} + +/** + * Sets or removes an attribute from an element. + * + * @param {HTMLElement} element - The element to set or remove attribute from. + * @param {string} qualifiedName - The name of the attribute. + * @param {false | string} value - The value of the attribute. When passing `false`, remove the attribute. + */ +function setOrRemoveAttribute(element: HTMLElement, qualifiedName: string, value: false | string): void { + if (value === false) { + element.removeAttribute(qualifiedName); + } else { + element.setAttribute(qualifiedName, value); + } +} + +/** + * Sets or removes an attribute from an element with an undo function. + * + * @param {HTMLElement} element - The element to set or remove attribute from. + * @param {string} qualifiedName - The name of the attribute. + * @param {false | string} value - The value of the attribute. When passing `false`, remove the attribute. + * + * @returns {() => void} The undo function, when called, will undo all manipulations by restoring values recorded at the time of the function call. + */ +function setOrRemoveAttributeWithUndo(element: HTMLElement, qualifiedName: string, value: false | string): () => void { + const prevValue = getAttribute(element, qualifiedName); + + setOrRemoveAttribute(element, qualifiedName, value); + + return () => setOrRemoveAttribute(element, qualifiedName, prevValue); +} + +/** + * Finds the first ancestor that fulfill the predicate. + * + * @param {HTMLElement} element - The starting element. This element will not be checked against the predicate. + * @param {(ancestor: HTMLElement) => boolean} predicate - The predicate to fulfill. + * + * @returns {HTMLElement | undefined} The first ancestor that fulfill the predicate, otherwise, `undefined`. + */ +function findAncestor(element: HTMLElement, predicate: (ancestor: HTMLElement) => boolean): HTMLElement | undefined { + let current = element.parentElement; + + while (current) { + if (predicate.call(element, current)) { + return current; + } + + current = current.parentElement; + } +} + +/** + * Indicates the action selected by performing a series of manipulations, with undo: + * + * - Accessibility: set `aria-pressed` to `true` + * - Applies `styleOptions.actionPerformedClassName` + * + * @param {HTMLElement[]} selectedActionElements - An array of elements that are representing the action and is selected. + * @param {string?} actionPerformedClassName - The name of the class to apply to all elements. + * + * @returns {() => void} The undo function, when called, will undo all manipulations by restoring values recorded at the time of the function call. + */ +function indicateActionSelectionWithUndo( + selectedActionElements: HTMLElement[], + actionPerformedClassName?: string +): (() => void) | undefined { + if (!selectedActionElements.length) { + return; + } + + // Verify all input elements are "ac-pushButton", could belongs to ActionSet or "card actions". + if (selectedActionElements.some(actionElement => !containsClassName(actionElement, 'ac-pushButton'))) { + console.warn( + 'botframework-webchat: Cannot mark selected action in the card, some elements are not an "ac-pushButton".' + ); + + return; + } + + // A distinct set of action set containers which has selections, excluding containers without actions. + // Multiple submission in an Adaptive Card is still a vague area and TBD. + // We might want to disable the whole card, just buttons in same container, or do nothing (today). + const actionSetElements = new Set(); + + selectedActionElements.forEach(selectedActionElement => { + const actionSetElement = findAncestor( + selectedActionElement, + ancestor => ancestor.getAttribute('role') === 'menubar' + ); + + actionSetElement && actionSetElements.add(actionSetElement); + }); + + const undoStack: (() => void)[] = []; + + actionSetElements.forEach(actionSetElement => { + // Remove "role" from every "ac-actionSet" container. + undoStack.push(setOrRemoveAttributeWithUndo(actionSetElement, 'role', false)); + + // Modify "role" of every actions in the container. + Array.from(actionSetElement.querySelectorAll('.ac-pushButton') as NodeListOf).forEach( + actionElement => { + if (selectedActionElements.includes(actionElement)) { + // Add "aria-pressed" and set "role" attribute to "button" (which is required by "aria-pressed"). + undoStack.push(setOrRemoveAttributeWithUndo(actionElement, 'aria-pressed', 'true')); + undoStack.push(setOrRemoveAttributeWithUndo(actionElement, 'role', 'button')); + + // Highlight actions by applying `styleOptions.actionPerformedClassName`. + actionPerformedClassName && + undoStack.push(addPersistentClassWithUndo(actionElement, actionPerformedClassName)); + } else { + // We removed "role=menubar" from the container, we must remove "role=menuitem" from unselected actions. + undoStack.push(setOrRemoveAttributeWithUndo(actionElement, 'role', false)); + } + } + ); + }); + + return () => undoStack.forEach(undo => undo()); +} + +/** + * Fixes accessibility issues from Adaptive Card, with undo. + * + * @returns {() => void} The undo function, when called, will undo all manipulations by restoring values recorded at the time of the function call. + */ +function fixAccessibilityIssuesWithUndo(element: HTMLElement): () => void { + // These hacks should be done in Adaptive Cards library instead. + const undoStack: (() => void)[] = []; + + // Related to #3949: All action buttons inside role="menubar" should be role="menuitem". + undoStack.push( + ...Array.from(element.querySelectorAll('.ac-actionSet[role="menubar"] [role="button"]')).map(actionButton => + setAttributeWithUndo(actionButton, 'role', 'menuitem') + ) + ); + + return () => undoStack.forEach(undo => undo()); +} + function getFocusableElements(element) { return [].filter.call( element.querySelectorAll( @@ -421,41 +582,28 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled adaptiveCard.onExecuteAction = disabled ? undefined : handleExecuteAction; }, [adaptiveCard, disabled, handleExecuteAction]); + useEffect(() => fixAccessibilityIssuesWithUndo(adaptiveCardElementRef.current), [adaptiveCardElementRef, lastRender]); + useEffect(() => { // If the Adaptive Card get re-rendered, re-disable elements as needed. if (disabled) { return disableInputElementsWithUndo(adaptiveCardElementRef.current); } - }, [disabled, lastRender]); + }, [adaptiveCardElementRef, disabled, lastRender]); useEffect(() => { // If the Adaptive Card changed, reset all actions performed. setActionsPerformed([]); }, [adaptiveCard]); - useEffect(() => { - // Add aria-pressed and role attribute to the AC action button selected by the user. - actionsPerformed.forEach(({ renderedElement }) => { - if (renderedElement && adaptiveCardElementRef.current.contains(renderedElement)) { - setAttributeWithUndo(renderedElement, 'aria-pressed', 'true'); - setAttributeWithUndo(renderedElement, 'role', 'button'); - } - }); - - // Add developers to highlight actions when they have been clicked. - if (!actionPerformedClassName) { - return; - } - - const undoStack = actionsPerformed.map( - ({ renderedElement }) => - renderedElement && - adaptiveCardElementRef.current.contains(renderedElement) && - addPersistentClassWithUndo(renderedElement, actionPerformedClassName) - ); - - return () => undoStack.forEach(undo => undo && undo()); - }, [actionsPerformed, actionPerformedClassName, lastRender]); + useEffect( + () => + indicateActionSelectionWithUndo( + actionsPerformed.map(({ renderedElement }) => renderedElement), + actionPerformedClassName + ), + [actionsPerformed, actionPerformedClassName, lastRender] + ); return errors.length ? ( node_env === 'development' && From 734b36a96ef065c9d0abb4b02b38c2e35d55f7bb Mon Sep 17 00:00:00 2001 From: William Wong Date: Wed, 16 Jun 2021 18:36:10 -0700 Subject: [PATCH 4/8] Update entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 575626e603..b1c753210d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixes [#3825](https://github.com/microsoft/BotFramework-WebChat/issues/3825). Add `role="presentation"` to all decorative ``, by [@compulim](https://github.com/compulim), in PR [#3903](https://github.com/microsoft/BotFramework-WebChat/pull/3903) - Fixes [#3360](https://github.com/microsoft/BotFramework-WebChat/issues/3360) and [#3615](https://github.com/microsoft/BotFramework-WebChat/issues/3615). Use `channelData['webchat:fallback-text']` field for screen reader text, before stripping Markdown from [`activity.text` field](https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#text), by [@compulim](https://github.com/compulim), in PR [#3917](https://github.com/microsoft/BotFramework-WebChat/pull/3917) - Fixes [#3856](https://github.com/microsoft/BotFramework-WebChat/issues/3856). Fix missing typings, by [@compulim](https://github.com/compulim) and [@corinagum](https://github.com/corinagum), in PR [#3931](https://github.com/microsoft/BotFramework-WebChat/pull/3931) -- Fixes [#3947](https://github.com/microsoft/BotFramework-WebChat/issues/3947). Adaptive Cards: all action sets (which has `role="menubar"`) must have at least 1 or more `role="menuitem"`, by [@compulim](https://github.com/compulim), in PR [#XXX](https://github.com/microsoft/BotFramework-WebChat/pull/XXX) +- Fixes [#3947](https://github.com/microsoft/BotFramework-WebChat/issues/3947). Adaptive Cards: all action sets (which has `role="menubar"`) must have at least 1 or more `role="menuitem"`, by [@compulim](https://github.com/compulim), in PR [#3950](https://github.com/microsoft/BotFramework-WebChat/pull/3950) ### Changed From 2a26b401b0967e7e69b85d136529daac5799e22d Mon Sep 17 00:00:00 2001 From: William Wong Date: Thu, 17 Jun 2021 12:55:14 -0700 Subject: [PATCH 5/8] Move to .ts --- .../src/hooks/internal/{useUniqueId.js => useUniqueId.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/component/src/hooks/internal/{useUniqueId.js => useUniqueId.ts} (100%) diff --git a/packages/component/src/hooks/internal/useUniqueId.js b/packages/component/src/hooks/internal/useUniqueId.ts similarity index 100% rename from packages/component/src/hooks/internal/useUniqueId.js rename to packages/component/src/hooks/internal/useUniqueId.ts From b56b30da04199762699bbfd71759dbe2143e2836 Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 21 Jun 2021 11:52:58 -0700 Subject: [PATCH 6/8] Add test file --- __tests__/html/accessibility.adaptiveCard.ariaPushed.js | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 __tests__/html/accessibility.adaptiveCard.ariaPushed.js diff --git a/__tests__/html/accessibility.adaptiveCard.ariaPushed.js b/__tests__/html/accessibility.adaptiveCard.ariaPushed.js new file mode 100644 index 0000000000..b63673be9c --- /dev/null +++ b/__tests__/html/accessibility.adaptiveCard.ariaPushed.js @@ -0,0 +1,6 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +describe('accessibility requirement', () => { + test('Adaptive Card should apply aria-pushed="true" after clicking a button', () => + runHTML('accessibility.adaptiveCard.ariaPushed.html')); +}); From 8c76f05a4bf7e6c27e1190370a9bb73f2c2f7e18 Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 29 Jun 2021 11:24:01 -0700 Subject: [PATCH 7/8] Fix test --- .../src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx index 84b18e6c26..b1b27dd1de 100644 --- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx +++ b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardRenderer.tsx @@ -599,7 +599,9 @@ const AdaptiveCardRenderer = ({ actionPerformedClassName, adaptiveCard, disabled useEffect( () => indicateActionSelectionWithUndo( - actionsPerformed.map(({ renderedElement }) => renderedElement), + // Actions that do not have "renderedElement" means it is the Adaptive Card itself, such as "selectAction" (AC) or "tapAction" (rich cards). + // We do not need to mark the whole card as performed. + actionsPerformed.map(({ renderedElement }) => renderedElement).filter(renderedElement => renderedElement), actionPerformedClassName ), [actionsPerformed, actionPerformedClassName, lastRender] From d3d24d3dce792067047e3185342cfad159d39c65 Mon Sep 17 00:00:00 2001 From: William Wong Date: Tue, 29 Jun 2021 12:02:58 -0700 Subject: [PATCH 8/8] Test reliability --- .../html/accessibility.adaptiveCard.withoutTapAction.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/__tests__/html/accessibility.adaptiveCard.withoutTapAction.html b/__tests__/html/accessibility.adaptiveCard.withoutTapAction.html index f86773cbf6..29623363f5 100644 --- a/__tests__/html/accessibility.adaptiveCard.withoutTapAction.html +++ b/__tests__/html/accessibility.adaptiveCard.withoutTapAction.html @@ -28,7 +28,11 @@ await host.sendShiftTab(); - expect(document.activeElement === pageElements.transcript()).toBe(true); + await pageConditions.became( + 'focus is on the transcript', + () => document.activeElement === pageElements.transcript(), + 1000 + ); // The card should be in white because it is not tappable, it should not be focused. // The transcript will become yellow because it is focused.