CreateAKSProject: Fix a11y issues, separate presentation, and add tests + stories + docs#357
Conversation
illume
force-pushed
the
create-project
branch
2 times, most recently
from
fb26003 to
9ef93be
Compare
illume
changed the title
illume
changed the title
illume
changed the title
There was a problem hiding this comment.
Pull request overview
Refactors the Create AKS Project wizard into a pure presentational component + stateful hook, while addressing multiple accessibility violations and adding Storybook stories and unit tests.
Changes:
- Introduces
useCreateAKSProjectWizardto centralize wizard state, side-effects, and submission flow. - Adds
CreateAKSProjectPure(and stories/tests) to separate presentation from logic and improve a11y. - Updates existing components/tests to match new ARIA patterns (e.g.,
role="status",aria-hiddenon spinners).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/aks-desktop/src/components/CreateAKSProject/hooks/useNamespaceCheck.ts | Adds gated debug logging for namespace checks. |
| plugins/aks-desktop/src/components/CreateAKSProject/hooks/useCreateAKSProjectWizard.ts | New hook encapsulating navigation, async creation flow, focus behavior, and CLI checks. |
| plugins/aks-desktop/src/components/CreateAKSProject/hooks/useCreateAKSProjectWizard.test.ts | Adds unit tests for wizard hook basics and submit success/error paths. |
| plugins/aks-desktop/src/components/CreateAKSProject/components/tests/ClusterConfigurePanel.test.tsx | Updates test expectations for new a11y loading announcement behavior. |
| plugins/aks-desktop/src/components/CreateAKSProject/components/SearchableSelect.tsx | Marks loading spinner as decorative for screen readers. |
| plugins/aks-desktop/src/components/CreateAKSProject/components/ClusterConfigurePanel.tsx | Adds role="status" live region + hides decorative spinner; adds aria-busy to button. |
| plugins/aks-desktop/src/components/CreateAKSProject/components/BasicsStep.tsx | Adds aria-busy and hides decorative spinners; gates debug logging. |
| plugins/aks-desktop/src/components/CreateAKSProject/tests/CreateAKSProjectPure.test.tsx | Adds interaction tests driven from Storybook story args. |
| plugins/aks-desktop/src/components/CreateAKSProject/CreateAKSProjectPure.tsx | New pure UI component with loading/error/success overlays and a11y attributes. |
| plugins/aks-desktop/src/components/CreateAKSProject/CreateAKSProjectPure.stories.tsx | Adds stories for default/loading/error/success/validation/loading-next scenarios. |
| plugins/aks-desktop/src/components/CreateAKSProject/CreateAKSProject.tsx | Converts to a thin connector composing the hook + pure component and mapping step content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks. I have some fixes coming. |
|
Fixed some more a11y issues. Some more stories and also guidepup tests were added to help test. Updated PR description.
"Keyboard access to access section doesn't work" is fixed
"Error message when deploying application is not read out" is fixed.
Yeah, I'm open to just removing it and the decoration. Did you mean remove the button and the decoration? Or you mean remove just the button? The A11y issues fixed in this round...
|
|
Going to fix the CI errors. |
illume
left a comment
illume
left a comment
There was a problem hiding this comment.
Cleaned up some warnings/console spam in new tests.
- Rebased against main, fixed breadcrumb conflict
- Fix Alertdialog so cannot be dismissed by backdrop/Escape
- Fix error dialog showed redacted message, only redact in logs
- Remove dead
!r.skippedcondition - Fix
inertattribute to not trigger a React 18 runtime warning
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vyncent-t
left a comment
vyncent-t
left a comment
There was a problem hiding this comment.
something I noticed:
3. Loading overlay (inert + aria-busy)
- narration is announced but very quickly (is cut off before new page loads)
vyncent-t
left a comment
vyncent-t
left a comment
There was a problem hiding this comment.
fixes are looking good, some more things I noticed
ReviewStep scrollable assignees region
- cannot get the section phrase to be announced, only the assignee inputs are announced
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
it looks like the whole section is now highlighted while being linked to the assignee input, but it should be fine if it is picked up by NVDA |
|
Thanks @vyncent-t for the testing and for answering my silly questions. haha |
|
I pushed another story state "ReviewStep -> Many assignees" to make it easier to test the scrollable assignee region with keyboard when there's multiple assignees. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pushed a fix for this ^ |
|
Rebased against main, fixed conflicts. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Added a test for code-merged-into-main for a bug with the onProgress not being aware it could be aborted. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s and stories
- CreateAKSProjectPure a11y fixes:
- `inert` on `CardContent` while loading — blocks keyboard focus from reaching controls hidden under the overlay (fixes axe `aria-hidden-focus`; `aria-hidden` alone does not block focus)
- `aria-busy` + `aria-describedby` on Card; `CircularProgress` with `aria-label`; status text with `aria-live="polite"`
- Error dialog: `role="alertdialog"` on `PaperProps` (fixes axe `aria-dialog-name` — setting it on the Dialog root goes to the wrong element); `autoFocus` on Cancel; `tabIndex={0}` on scrollable `DialogContent` (fixes axe `scrollable-region-focusable`)
- Success dialog: `autoFocus` on Application name input; valid heading hierarchy; decorative icons marked `aria-hidden="true"`
- Breadcrumb: `role="navigation"` + `aria-label`; `role="button"` + `tabIndex={0}` + keyboard handler on step items; `aria-current="step"` on active step
- Next button: `aria-busy` while Azure resources load; spinner inside button has `aria-hidden="true"`
- BaseStep: fixes for project edit button
- Storybook stories for all states: basics, loading overlay, error (short/long/scrollable), validation, success (empty/filled), next-loading
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- plugins/aks-desktop/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
skoeva
left a comment
skoeva
left a comment
There was a problem hiding this comment.
some thoughts at first glance:
- title should be capitalized
- no subtitle like on the other pages
- no back button at the top (page needs
backLinkprop)
- why is "create new project" a window and "create new project from yaml" a separate page? maybe these should be aligned better or consolidated somehow
|
Thanks @skoeva These ones are in main branch as well... I think probably they should be in a later PR. |
|
right, these bugs are not necessarily in scope for this PR - they aren't blocking |
|
Nab nav testing done for each of the options under "Create project" modal
While not functionally broken, the namespace options do have some issues with missing translation keys Posted screenshot images within the group chat (see those) |
Description
Fixes accessibility violations in the Create AKS Project wizard (
CreateAKSProjectPure), separates logic into to hooks and adds new Pure presentation components, also adds tests + stories + docs.Fixes #300
Fixes #305
Fixes #94
Fixes #310
Type of Change
Related Issues
Changes Made
CreateAKSProjectPurea11y fixes:inertonCardContentwhile loading — blocks keyboard focus from reaching controls hidden under the overlay (fixes axearia-hidden-focus;aria-hiddenalone does not block focus)aria-busy+aria-describedbyon Card;CircularProgresswitharia-label; status text witharia-live="polite"role="alertdialog"onPaperProps(fixes axearia-dialog-name— setting it on the Dialog root goes to the wrong element);autoFocuson Cancel;tabIndex={0}on scrollableDialogContent(fixes axescrollable-region-focusable)autoFocuson Application name input; valid heading hierarchy; decorative icons markedaria-hidden="true"role="navigation"+aria-label;role="button"+tabIndex={0}+ keyboard handler on step items;aria-current="step"on active steparia-busywhile Azure resources load; spinner inside button hasaria-hidden="true"A11y Fixes (each with MDN/MUI/Deque documentation link in code comment):
Breadcrumb.tsx: Added'&:focus-visible'outline usingtheme.palette.primary.main(not the sx shorthand'primary.main', which MUI does not palette-transform foroutlineColor),borderRadius: 1, andaria-hidden="true"on each step<Icon>CreateAKSProjectPure.tsx: Addedrole="alert"on errorTypography;role="status"on success descriptionTypographyResourceCard.tsx: Addedaria-hidden="true"on decorative titleIconComputeStep.tsx: Addedaria-hidden="true"on allstartAdornmentIcons (arrow-up/down)ReviewStep.tsx: Addedaria-hidden="true"on all 6 decorative section Icons;tabIndex={0}+role="region"+aria-labelon scrollable assigneesBoxFormField.tsx: AddedFormHelperTextPropswitharia-disabledwhendisabledTest Fixes:
CreateAKSProjectPure.test.tsx: Fixed@iconify/reactmock to pass through all props via{...props}(matching theBasicsStep.test.tsxpattern) instead of hard-codingaria-hidden="true", so icon a11y assertions are not false-positives. Renamed misleading test from "visible :focus-visible outline" to "keyboard-reachable (tabIndex is not -1)" to match what it actually asserts. Scoped breadcrumb iconaria-hiddenassertion to thenavigationlandmark element.CreateAKSProjectPure.guidepup.test.tsx: Same@iconify/reactmock fix — passes through all props instead of defaultingaria-hiddento'true'.Stories Added (37+ stories total across 9 files):
CreateAKSProjectPure.stories.tsx— 9 stories: BasicsStepDefault, NextButtonLoading, ValidationError, LoadingOverlay, ErrorOverlay, ErrorOverlayScrollable, LongErrorMessage, SuccessDialog, SuccessDialogWithAppNameNetworkingStep.stories.tsx— 4 stories: Default, DenyAll, AllowAll, LoadingComputeStep.stories.tsx— 4 stories: Default, WithFieldErrors, CpuRequestError, LoadingReviewStep.stories.tsx— 5 stories: FullConfiguration, NoAssignees, NoDescription, UnresolvedResources, SingleAssigneeAccessStep.stories.tsx— 5 stories: Default, MultipleAssignees, NoAssignees, InvalidEmail, LoadingBreadcrumb.stories.tsx— 3 stories: FirstStep, MiddleStep, LastStepFormField.stories.tsx— 5 stories: Default, WithError, NumberField, Disabled, MultilineSearchableSelect.stories.tsx— 5 stories: Default, WithSelection, Loading, WithError, DisabledValidationAlert.stories.tsx— 6 stories: Error, Warning, Success, Info, WithAction, HiddenGuidepup Virtual Screen Reader Tests (~150 tests):
CreateAKSProjectPure: breadcrumb nav, step buttons, aria-current, hidden icons, Cancel/Next, validation disabled, loading busy, overlay progressbar, alertdialog + role=alert, success dialog + role=status, step navigationNetworkingStep: heading, intro text, Ingress/Egress comboboxes, current values, disabled stateComputeStep: CPU/Memory headings, spinbutton values + helper text, invalid/not-invalid state, error isolation, disabled loadingReviewStep: h3 sections, resolved names, description fallback, assignee count, N/A degradationAccessStep: empty state, invalid email, valid email, loading stateBreadcrumb: nav landmark, aria-current across step positionsFormField: textbox/spinbutton roles, invalid state, disabled state, error helper textSearchableSelect: combobox role, selected value announcementValidationAlert: alert role with message content, hidden renders nothingNote: there is a code comment for each a11y aria fix.
Testing
Manual A11y Testing Steps
1. Keyboard — wizard navigation
Cancel → (step form fields) → Next — in that order, with nothing skipped or trapped.
2. Keyboard — breadcrumb navigation
3. Loading overlay (
inert+aria-busy)inertblocks it).aria-live="polite"on the status text).aria-busy="true"is set.4. Error alertdialog (
role="alertdialog"+autoFocus+ scrollable region)role="alertdialog",aria-labelledby, andaria-describedbyare all set.5. Success dialog (
autoFocus+ focus trap)aria-labelledbyandaria-describedbypoint to the title and description elements.6. Decorative icons (
aria-hidden)aria-hidden="true".7. Repro Steps from "Unable to scroll failure / error message using keyboard"
To reproduce:
8. Repro Steps from "Narrator/NVDA does not announce the failure message after submitting the Create Project request, when the operation fails"
To reproduce:
9. Repro Steps from "Edit Button is not functioning on the Create New page".
Note: edit button was removed. The edit icon is aria-hidden now.
10. Breadcrumb focus ring (keyboard navigation)
11. Error dialog screen reader announcement
role="alert"(assertive live region)role="alert"12. Success dialog screen reader announcement
role="status"(polite live region)role="status"13. Decorative icons hidden from screen readers
aria-hidden="true"<svg>or icon<span>and confirmaria-hidden="true"is present14. ReviewStep scrollable assignees region
15. Disabled field helper text (no false contrast failure)**
aria-disabledattribute tells axe to skip the contrast check per WCAG 1.4.3 inactive UI exemptionScreenshots/Videos
CreateAKSProjectPure (main wizard)
role="alert")role="status")Step Components
Shared Components
Checklist
Breaking Changes
None.
Performance Impact
Documentation Updates