Skip to content

Keyboard accessibility bugs (463, 853, 593)#2699

Merged
mhdahman merged 3 commits intomasterfrom
mhdahman/a11y-keyboard
Apr 6, 2023
Merged

Keyboard accessibility bugs (463, 853, 593)#2699
mhdahman merged 3 commits intomasterfrom
mhdahman/a11y-keyboard

Conversation

@mhdahman
Copy link
Contributor

@mhdahman mhdahman commented Mar 31, 2023 •
edited
Loading

Resolves:

  • [Bug 853]: [Keyboard Navigation-Batch Explorer-SetUp]: Keyboard focus does not land on the first interactive control as soon as we invoke the 'Setup' control in Storage accounts pane.
  • [Bug 463]: [Keyboard Navigation -Batch Explorer - Job preparation task] After activating ‘Job preparation task’ keyboard focus does not lands on first interactive control.
  • [Bug 593] [Keyboard Navigation-Batch Explorer-Gallery-V-Ray] Visual focus indicator does not moves to the "run job with auto pool" button.

Notable changes:

  • Updated the bl-complex-form implementation so that it automatically focuses the first element in any nested-forms when they open. As a result, this fixes all instances of bl-complex-form that had similar auto focus issues, not just the "Job preparation task" form in the bug above. For example, a similar issue was found in the "Job manager task" nested form, which should be fixed now as well.
  • Fixed runtime error causing keyboard tab navigation to stop working in the Storage accounts pane mentioned above

mhdahman
mhdahman commented Mar 31, 2023
View reviewed changes
src/@batch-flask/ui/form/complex-form/complex-form.component.ts
Comment on lines +278 to +280
private focusFirstFocusableElement() {
this.formElement.nativeElement?.querySelector('[autofocus], button, input, textarea, select')?.focus()
}
Copy link
Contributor Author

@mhdahman mhdahman Mar 31, 2023

Choose a reason for hiding this comment

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

The existing autofocus directive will not work as expected when used on an element nested inside of complex-form because those elements seem to be injected/rendered in the DOM before they're visible/shown. We'll have to explicitly focus those elements after the nested form page is opened.

@codecov
Copy link

codecov bot commented Mar 31, 2023 •
edited
Loading

Codecov Report

Merging #2699 (b2513a1) into master (8ec7a70) will increase coverage by 0.00%.
The diff coverage is 77.77%.

❗ Current head b2513a1 differs from pull request most recent head 338a5a1. Consider uploading reports for the commit 338a5a1 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2699   +/-   ##
=======================================
  Coverage   65.08%   65.09%           
=======================================
  Files         916      916           
  Lines       26468    26475    +7     
  Branches     5233     5237    +4     
=======================================
+ Hits        17226    17233    +7     
  Misses       9242     9242
Impacted Files Coverage Δ
...nt-picker/auto-storage-account-picker.component.ts 73.68% <0.00%> (ø)
...ask/ui/form/complex-form/complex-form.component.ts 64.39% <100.00%> (+1.99%) ⬆️

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 8ec7a70...338a5a1. Read the comment docs.

@mhdahman mhdahman force-pushed the mhdahman/a11y-keyboard branch from f03f6b1 to afc1a9b Compare March 31, 2023 18:17
cRui861
cRui861 previously approved these changes Apr 4, 2023
View reviewed changes
@mhdahman mhdahman force-pushed the mhdahman/a11y-keyboard branch from afc1a9b to 169e0b6 Compare April 5, 2023 20:32
@mhdahman
Copy link
Contributor Author

mhdahman commented Apr 5, 2023 •
edited
Loading

Pushed new changes to add a fix for [Bug 593] [Keyboard Navigation-Batch Explorer-Gallery-V-Ray] Visual focus indicator does not moves to the "run job with auto pool" button.

@mhdahman mhdahman force-pushed the mhdahman/a11y-keyboard branch from 0c90e7c to 542aba7 Compare April 5, 2023 21:48
@mhdahman mhdahman changed the title Keyboard accessibility bugs (463, 853) Keyboard accessibility bugs (463, 853, 593) Apr 5, 2023
@mhdahman mhdahman requested a review from cRui861 April 5, 2023 21:48
@mhdahman mhdahman force-pushed the mhdahman/a11y-keyboard branch from 542aba7 to 338a5a1 Compare April 6, 2023 18:44
@mhdahman mhdahman merged commit d1abd51 into master Apr 6, 2023
@mhdahman mhdahman deleted the mhdahman/a11y-keyboard branch April 6, 2023 20:12
@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

@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

@cRui861 cRui861 Awaiting requested review from cRui861 cRui861 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