Skip to content

Accessibility bug fixes (865, 950, 990, 702, 671)#2704

Merged
gingi merged 5 commits intomasterfrom
mhdahman/a11y-focus-bugs
Apr 18, 2023
Merged

Accessibility bug fixes (865, 950, 990, 702, 671)#2704
gingi merged 5 commits intomasterfrom
mhdahman/a11y-focus-bugs

Conversation

@mhdahman
Copy link
Contributor

@mhdahman mhdahman commented Apr 7, 2023 •
edited
Loading

Resolves:

  • [865] [Keyboard Navigation-Azure Batch Explorer-Add a Pool]: Visual focus indicator is not visible for the multiple controls, while navigating using Tab key.
  • [950] [Screen Readers-Azure Batch Explorer-Job>>Disable]: Visual focus indicator is not visible for the 'radio' buttons, while navigating using TAB key under the disable dialog.
  • [990] [Screen Reader-Batch Explorer-Add button]: Focus moves to background while "Create a new empty container" pop-up is open.
  • [702] [Keyboard Navigation - Batch Explore - Pools]: Focus order is not logical after refresh control under "Node" on "Pools" page.
  • [671] [Keyboard Navigation - Batch Explore - Add a batch account]: While closing the subscription dropdown whole dialog is getting close with esc key.

Notable changes:

  • keydown events handled by the select component no longer propagates
  • Square buttons with primary colors now have a white focus ring
    Screenshot 2023-04-07 at 3 18 20 PM
  • Style improvements for focused and selected states of mat-radio-button
    Screenshot 2023-04-07 at 3 19 26 PM

@codecov
Copy link

codecov bot commented Apr 7, 2023 •
edited
Loading

Codecov Report

Merging #2704 (34eb673) into master (c8192c1) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 34eb673 differs from pull request most recent head 35e32da. Consider uploading reports for the commit 35e32da to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2704      +/-   ##
==========================================
- Coverage   65.08%   65.08%   -0.01%     
==========================================
  Files         916      916              
  Lines       26477    26478       +1     
  Branches     5237     5237              
==========================================
  Hits        17233    17233              
- Misses       9244     9245       +1
Impacted Files Coverage Δ
src/@batch-flask/ui/select/select.component.ts 82.80% <0.00%> (-0.30%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8192c1...35e32da. Read the comment docs.

mhdahman
mhdahman commented Apr 7, 2023
View reviewed changes
src/app/components/data/home/data-home.html
Comment on lines 6 to +7
<bl-button type="plain" icon="fa fa-plus spin-hover" color="light" title="Add a file group" [matMenuTriggerFor]="addFileGroupMenu"
[disabled]="!hasAutoStorage" [@.disabled]="true">
(click)="addFileGroup()" (do)="addFileGroup()" [disabled]="!hasAutoStorage" [@.disabled]="true">
Copy link
Contributor Author

@mhdahman mhdahman Apr 7, 2023

Choose a reason for hiding this comment

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

The bl-button doesn't play nicely with the mat-menu component out of the box when used a trigger as I couldn't activate the trigger using the keyboard (at least on macOS). Mouse clicks seem fine.

mat-menu expects an actual button component to be used as a trigger, but here bl-button is a div not a button element. I had to add click and do above in order to call trigger.openMenu() programmatically.

@gingi gingi force-pushed the mhdahman/a11y-focus-bugs branch from c3ac7d8 to 35e32da Compare April 18, 2023 16:29
@gingi gingi enabled auto-merge April 18, 2023 16:31
@gingi gingi merged commit 9f4c85b into master Apr 18, 2023
@gingi gingi deleted the mhdahman/a11y-focus-bugs branch April 18, 2023 16:57
@cRui861
Copy link
Member

cRui861 commented Apr 26, 2023

Fixes #2713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gingi gingi gingi approved these changes

@cRui861 cRui861 Awaiting requested review from cRui861 cRui861 is a code owner

@dpwatrous dpwatrous Awaiting requested review from dpwatrous dpwatrous is a code owner

@zfengms zfengms Awaiting requested review from zfengms zfengms is a code owner

@paterasMSFT paterasMSFT Awaiting requested review from paterasMSFT

@seth-barshay seth-barshay Awaiting requested review from seth-barshay

@wiboris wiboris Awaiting requested review from wiboris wiboris is a code owner

@skapur12 skapur12 Awaiting requested review from skapur12 skapur12 is a code owner

@NickKouds NickKouds Awaiting requested review from NickKouds NickKouds is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mhdahman @cRui861 @gingi