diff --git a/.changelog/5900-fix-card-action-pressed-state.md b/.changelog/5900-fix-card-action-pressed-state.md new file mode 100644 index 0000000000..cb49f040ee --- /dev/null +++ b/.changelog/5900-fix-card-action-pressed-state.md @@ -0,0 +1,7 @@ +### Changed + +- 💥 Card action buttons the user pressed are styled with the `adaptive-card-renderer__action--selected` class instead of `[aria-pressed='true']`: host CSS that targets `.ac-pushButton[aria-pressed='true']` should target `.ac-pushButton.adaptive-card-renderer__action--selected` instead, in PR [#5900](https://github.com/microsoft/BotFramework-WebChat/pull/5900), by [@OEvgeny](https://github.com/OEvgeny) + +### Fixed + +- Fixed [#4583](https://github.com/microsoft/BotFramework-WebChat/issues/4583), [#5716](https://github.com/microsoft/BotFramework-WebChat/issues/5716), [#5719](https://github.com/microsoft/BotFramework-WebChat/issues/5719), [#5745](https://github.com/microsoft/BotFramework-WebChat/issues/5745), and [#5750](https://github.com/microsoft/BotFramework-WebChat/issues/5750), card action buttons being announced as toggle buttons, and Submit being marked as selected when the card's inputs fail validation, in PR [#5900](https://github.com/microsoft/BotFramework-WebChat/pull/5900), by [@OEvgeny](https://github.com/OEvgeny) diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html index 27bf6b2477..35b413158d 100644 --- a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html +++ b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html @@ -1,4 +1,4 @@ - + @@ -8,30 +8,29 @@
- + diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-1.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-1.png new file mode 100644 index 0000000000..acdacd1c44 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-1.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-2.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-2.png new file mode 100644 index 0000000000..d934e421db Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-2.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-3.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-3.png new file mode 100644 index 0000000000..6863240ebb Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-3.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-4.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-4.png new file mode 100644 index 0000000000..0d59335acc Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-4.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-5.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-5.png new file mode 100644 index 0000000000..054180e40d Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-5.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-6.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-6.png new file mode 100644 index 0000000000..920758bba3 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-6.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-7.png b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-7.png new file mode 100644 index 0000000000..32419ec2ee Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/ariaPushed.html.snap-7.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/hack.actionShouldBePushButtonMod.html b/__tests__/html2/accessibility/adaptiveCard/hack.actionShouldBePushButtonMod.html index af4732612a..1f15b2d1f3 100644 --- a/__tests__/html2/accessibility/adaptiveCard/hack.actionShouldBePushButtonMod.html +++ b/__tests__/html2/accessibility/adaptiveCard/hack.actionShouldBePushButtonMod.html @@ -9,6 +9,10 @@
diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html b/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html new file mode 100644 index 0000000000..56ff19130f --- /dev/null +++ b/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html @@ -0,0 +1,9 @@ + + + + + + + diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html.snap-1.png b/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html.snap-1.png new file mode 100644 index 0000000000..ee8eca8567 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/selectedAction.copilot.html.snap-1.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html b/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html new file mode 100644 index 0000000000..e9a9a1df85 --- /dev/null +++ b/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html @@ -0,0 +1,9 @@ + + + + + + + diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html.snap-1.png b/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html.snap-1.png new file mode 100644 index 0000000000..95f390d541 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/selectedAction.fluent.html.snap-1.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.html b/__tests__/html2/accessibility/adaptiveCard/selectedAction.html new file mode 100644 index 0000000000..5a554f8a4a --- /dev/null +++ b/__tests__/html2/accessibility/adaptiveCard/selectedAction.html @@ -0,0 +1,167 @@ + + + + + + +
+ + + + diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.html.snap-1.png b/__tests__/html2/accessibility/adaptiveCard/selectedAction.html.snap-1.png new file mode 100644 index 0000000000..1655766080 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/selectedAction.html.snap-1.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html new file mode 100644 index 0000000000..7ff391135c --- /dev/null +++ b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html @@ -0,0 +1,155 @@ + + + + + + +
+ + + + diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-1.png b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-1.png new file mode 100644 index 0000000000..4de945bb9b Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-1.png differ diff --git a/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-2.png b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-2.png new file mode 100644 index 0000000000..168fcc4b82 Binary files /dev/null and b/__tests__/html2/accessibility/adaptiveCard/selectedAction.validation.html.snap-2.png differ diff --git a/__tests__/html2/richCard/heroCard.pushButton.focusVisible.html b/__tests__/html2/richCard/heroCard.pushButton.focusVisible.html index bbad013e64..b3f8f35189 100644 --- a/__tests__/html2/richCard/heroCard.pushButton.focusVisible.html +++ b/__tests__/html2/richCard/heroCard.pushButton.focusVisible.html @@ -97,13 +97,15 @@ await pageConditions.numActivitiesShown(2); await pageConditions.allOutgoingActivitiesSent(); - // THEN: The "Places To Buy" button should be pressed and keep the focus. - await waitFor(() => expect(placesToBuyButton.getAttribute('aria-pressed')).toBe('true')); + // THEN: The "Places To Buy" button should be selected and keep the focus. + await waitFor(async () => + expect((await testHelpers.getAccessibilityNode(placesToBuyButton)).description?.value).toBe('Selected') + ); expect(document.activeElement).toBe(placesToBuyButton); await pageConditions.scrollToBottomCompleted(); - // THEN: The pressed "Places To Buy" button (blue) keeps the focus indicator, the white band separates it from the black ring. + // THEN: The selected "Places To Buy" button (blue) keeps the focus indicator, the white band separates it from the black ring. // THEN: The "Place to buy" message is sent below the carousel. await host.snapshot('local'); }); diff --git a/packages/api/src/localization/en-US.json b/packages/api/src/localization/en-US.json index 4a978b61e5..de235be977 100644 --- a/packages/api/src/localization/en-US.json +++ b/packages/api/src/localization/en-US.json @@ -47,6 +47,8 @@ "ACTIVITY_STATUS_TIMESTAMP_ONE_MINUTE_AGO": "A minute ago", "ACTIVITY_STATUS_TIMESTAMP_TODAY": "Today", "ACTIVITY_STATUS_TIMESTAMP_YESTERDAY": "Yesterday", + "ADAPTIVE_CARD_ACTION_SELECTED_ALT": "Selected", + "_ADAPTIVE_CARD_ACTION_SELECTED_ALT.comment": "This is for screen reader only. It is the description of a button in a card after the user pressed it, e.g. \"Snooze, button, Selected\".", "ADAPTIVE_CARD_ERROR_BOX_TITLE_PARSE": "Adaptive Card parse error", "ADAPTIVE_CARD_ERROR_BOX_TITLE_RENDER": "Adaptive Card render error", "ANIMATION_PAUSE_BUTTON_ALT": "Pause animation", diff --git a/packages/bundle/src/adaptiveCards/AdaptiveCardsStyleOptions.ts b/packages/bundle/src/adaptiveCards/AdaptiveCardsStyleOptions.ts index 4996648031..4a7f195865 100644 --- a/packages/bundle/src/adaptiveCards/AdaptiveCardsStyleOptions.ts +++ b/packages/bundle/src/adaptiveCards/AdaptiveCardsStyleOptions.ts @@ -11,12 +11,12 @@ type StrictAdaptiveCardsStyleOptions = { cardEmphasisBackgroundColor: string | undefined; /** - * Adaptive Cards: background color of Adaptive Cards button with status of 'aria-pressed' + * Adaptive Cards: background color of the selected Adaptive Cards button, i.e. the action the user has performed */ cardPushButtonBackgroundColor: string | undefined; /** - * Adaptive Cards: text color of Adaptive Cards button with status of 'aria-pressed', and the inner band of the keyboard focus indicator of Adaptive Cards buttons + * Adaptive Cards: text color of the selected Adaptive Cards button, i.e. the action the user has performed, and the inner band of the keyboard focus indicator of Adaptive Cards buttons */ cardPushButtonTextColor: string | undefined; diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/private/closest.ts b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/private/closest.ts deleted file mode 100644 index fdbda5f5f5..0000000000 --- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/private/closest.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Ponyfill `HTMLElement.closest`. -// https://developer.mozilla.org/en-US/docs/Web/API/Element/closest -export default function closest(element: HTMLElement, selector: string): HTMLElement | undefined { - if (typeof element.closest === 'function') { - return element.closest(selector); - } - - let current: HTMLElement | null = element; - - while (current) { - // "msMatchesSelector" is vendor-prefixed version of "matches". - if ((current.matches || (current['msMatchesSelector'] as (selector: string) => boolean)).call(current, selector)) { - return current; - } - - current = current.parentElement; - } -} diff --git a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/useActionShouldBePushButtonModEffect.ts b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/useActionShouldBePushButtonModEffect.ts index 6a662d04d4..be220b040b 100644 --- a/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/useActionShouldBePushButtonModEffect.ts +++ b/packages/bundle/src/adaptiveCards/Attachment/AdaptiveCardHacks/useActionShouldBePushButtonModEffect.ts @@ -1,10 +1,8 @@ -import { useMemo, useRef } from 'react'; +import { useCallback, useMemo, useRef } from 'react'; -import addEventListenerWithUndo from '../../DOMManipulationWithUndo/addEventListenerWithUndo'; import bunchUndos from '../../DOMManipulationWithUndo/bunchUndos'; import durableAddClassWithUndo from '../../DOMManipulationWithUndo/durableAddClassWithUndo'; import setOrRemoveAttributeIfFalseWithUndo from '../../DOMManipulationWithUndo/setOrRemoveAttributeIfFalseWithUndo'; -import closest from './private/closest'; import findDOMNodeOwner from './private/findDOMNodeOwner'; import useAdaptiveCardModEffect from './private/useAdaptiveCardModEffect'; import usePrevious from './private/usePrevious'; @@ -13,93 +11,85 @@ import type { AdaptiveCard, CardObject } from 'adaptivecards'; import type { UndoFunction } from '../../DOMManipulationWithUndo/types/UndoFunction'; /** - * Accessibility: Action in ActionSet/CardElement should be push button. + * Accessibility: Action in ActionSet/CardElement should be push button, i.e. a plain command button. * * Pressing the action button is a decision-making process. The decision made by the end-user need to be read by the screen reader. * Thus, we need to indicate what decision the end-user made. * - * Since action buttons are button, the intuitive way to indicate selection of a button is marking it as pressed. + * Action buttons are not toggle buttons: pressing a selected action again does not unselect it. + * Thus, instead of `aria-pressed`, the selected action is described by the element of `descriptionId` (e.g. "Selected"), + * and styled by `selectedClassName`. * - * One exception is the `Action.ShowUrl` action. This button represents expand/collapse header of an accordion. - * Thus, their state is indicated by `aria-expanded`, instead of `aria-pressed`. + * An action is selected only after Adaptive Cards executed it, i.e. `markActionAsSelected()` is called from `onExecuteAction`. + * Adaptive Cards does not execute an action when its inputs fail validation, e.g. a required input is empty. + * + * One exception is the `Action.ShowCard` action. This button represents expand/collapse header of an accordion. + * Thus, their state is indicated by `aria-expanded` and they are never selected. * However, we still need to remove other unnecessary ARIA fields. */ export default function useActionShouldBePushButtonModEffect( adaptiveCard: AdaptiveCard -): readonly [(cardElement: HTMLElement, actionPerformedClassName?: string | undefined) => void, () => void] { +): readonly [ + (cardElement: HTMLElement, selectedClassName: string, descriptionId: string) => void, + () => void, + (action: CardObject) => void +] { + const markActionAsSelectedRef = useRef<((action: CardObject) => void) | undefined>(); const prevAdaptiveCard = usePrevious(adaptiveCard); const pushedCardObjectsRef = useRef>(new Set()); prevAdaptiveCard === adaptiveCard || pushedCardObjectsRef.current.clear(); const modder = useMemo( - () => (adaptiveCard: AdaptiveCard, cardElement: HTMLElement, actionPerformedClassName?: string | undefined) => { - const undoStack: UndoFunction[] = []; - - Array.from(cardElement.querySelectorAll('button.ac-pushButton') as NodeListOf).forEach( - actionElement => { - const cardObject = findDOMNodeOwner(adaptiveCard, actionElement); - - if (!actionElement.hasAttribute('aria-expanded')) { - if (pushedCardObjectsRef.current.has(cardObject)) { - actionPerformedClassName && - undoStack.push(durableAddClassWithUndo(actionElement, actionPerformedClassName)); - - undoStack.push(setOrRemoveAttributeIfFalseWithUndo(actionElement, 'aria-pressed', 'true')); - } else { - undoStack.push(setOrRemoveAttributeIfFalseWithUndo(actionElement, 'aria-pressed', 'false')); - } - } - - undoStack.push( - setOrRemoveAttributeIfFalseWithUndo(actionElement, 'aria-posinset', false), - setOrRemoveAttributeIfFalseWithUndo(actionElement, 'aria-setsize', false), - setOrRemoveAttributeIfFalseWithUndo(actionElement, 'role', false) - ); - } + () => (adaptiveCard: AdaptiveCard, cardElement: HTMLElement, selectedClassName: string, descriptionId: string) => { + const actionElements = Array.from( + cardElement.querySelectorAll('button.ac-pushButton') as NodeListOf ); + const undoStack: UndoFunction[] = []; - undoStack.push( - addEventListenerWithUndo( - cardElement, - 'click', - ({ target }) => { - // Depends on click location, `target` could be the
inside the