Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Designer][Accessibility] Put open showcard next in tab order #5424

Merged
merged 6 commits into from
Apr 28, 2021

Conversation

golddove
Copy link
Member

@golddove golddove commented Feb 24, 2021

Related Issue

ADO 30866987

Description

For keyboard users, an ActionSet with multiple Action.ShowCard's could cause an issue with tab order: after a user selects a ShowCard to activate, they may have to tab through several other Actions in the ActionSet before they reach the newly revealed content. This temporarily removes those Actions from the taborder after a card is revealed (then resets them after tabbing).

Seems to fall under this WCAG failure condition:
F85: Failure of Success Criterion 2.4.3 due to using dialogs or menus that are not adjacent to their trigger control in the sequential navigation order

Sample Card

Scenarios/FoodOrder

How Verified

Manual verification

Microsoft Reviewers: Open in CodeFlow

@ghost
Copy link

ghost commented Feb 24, 2021

Hi @golddove. Thanks for helping make the AdaptiveCards JS renderer + tooling better. As additional verification, once the JS build succeeds, please go to the test site to test out your website/designer changes.

@dclaux
Copy link
Member

dclaux commented Feb 24, 2021

I don't believe this is the right way to handle this.

Currently, one navigates from one button to another in a set using the Tab key, thus indeed a user needs to tab through all buttons before they can land in the open ShowCard.

We should change so that the navigation in the button strip is done via arrow keys. The full button strip would be a tab stop, but the next tab would go to to the next focusable element in the card, namely the open card.

@dclaux
Copy link
Member

dclaux commented Feb 24, 2021

We probably want to have a design conversation about this.

Copy link
Member

@dclaux dclaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is the right way to handle the requirement. Let's have a design discussion.

@golddove golddove added the Design Discussion - Needed Issue requires a design discussion before proceeding label Mar 1, 2021
@ghost ghost removed the Needs: Author Feedback label Mar 1, 2021
@golddove golddove added this to the 21.03 milestone Mar 1, 2021
@ghost ghost added the no-recent-activity label Mar 6, 2021
@ghost ghost assigned shalinijoshi19 Mar 6, 2021
@ghost
Copy link

ghost commented Mar 6, 2021

Hi @golddove. This non-spec pull request has had no recent activity for the past 5 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

@ghost ghost removed the no-recent-activity label Mar 29, 2021
@ghost
Copy link

ghost commented Mar 29, 2021

Hi @golddove; Thanks for taking action on your previously stale pull request. Resetting staleness.

@@ -4544,12 +4558,28 @@ class ActionCollection {
}

private expandShowCardAction(action: ShowCardAction, raiseEvent: boolean) {
let afterSelectedAction = false;
for (let button of this.buttons) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line before if

for (let button of this.buttons) {
console.log(button);
console.log(afterSelectedAction);
// remove all following actions from tabOrder, to skip focus directly to expanded card
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line before comment

if (afterSelectedAction) {
button.focusable = false;
console.log(button.focusable);
}
if (button.action !== action) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line before if

afterSelectedAction = true;
}
}
if(action.renderedElement) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line before if

@ghost ghost added the no-recent-activity label Apr 3, 2021
@ghost
Copy link

ghost commented Apr 3, 2021

Hi @golddove. This non-spec pull request has had no recent activity for the past 5 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

@RebeccaAnne RebeccaAnne modified the milestones: 21.03, Short Term Apr 7, 2021
@ghost ghost removed the no-recent-activity label Apr 27, 2021
@ghost
Copy link

ghost commented Apr 27, 2021

Hi @golddove; Thanks for taking action on your previously stale pull request. Resetting staleness.

@golddove golddove modified the milestones: Short Term, 21.04 Apr 27, 2021
@golddove golddove merged commit f6ef719 into main Apr 28, 2021
@golddove golddove deleted the golddove/30866987 branch April 28, 2021 06:23
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
…oft#5424)

* Put open showcard next in tab order

* Remove logs

* Empty line

* Fix merge

Co-authored-by: Paul Campbell <paulcam@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Discussion - Needed Issue requires a design discussion before proceeding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants