feat: Add accessibility scenarios for Fluent UI v9 components #3 #23334
Conversation
Co-authored-by: Peter Varholak <peter.varholak@gmail.com>
…c/fluentui into accessibility-scenarios
…d Tooltip scenario and correct some wording
…c/fluentui into accessibility-scenarios
…utton component in Tooltip scenario
| value={message} | ||
| style={messageStyle} | ||
| /> | ||
| <Button disabledFocusable={sendButtonDisabled} onClick={onSendButtonClick}> |
There was a problem hiding this comment.
This is an interesting thing to test, but it feels pretty weird to have the send/delete buttons be disabled and focusable. If this is just used to gather feedback from users, it seems fine. But if we plan on having this on our public docs, we should probably update it so the send button at least is always enabled.
There was a problem hiding this comment.
It's not so easy to find a case when disabledFocusable would make sense, but you are right here... I have made the "Send" button enabled at all times. Do you think I could leave the "Delete" button disabledFocusable at least?
There was a problem hiding this comment.
How about making it tabindex="-1" when disabled and then that way if it becomes disabled because someone clicks it, keyboard focus doesn't shift, but it's still not in the tab order (unless enabled). I think that'd be a pretty good real-world use case.
There was a problem hiding this comment.
That sounds like a good idea. So should I make the following changes?
- Leave the "Send" button always enabled.
- Make the "Delete" button disabled when activated, but set tabindex="-1" so that it remains in focus after becoming disabled.
- If there are no more steps for the "Decrease font size" or "Increase font size" buttons, make them disabledFocusable instead of disabled.
There was a problem hiding this comment.
So I've tried setting tabindex="-1" on the "Delete" button after clicking it, where clicking it also makes it disabled, but I cannot set focus programatically on the button afterwards, because it is not possible to set focus programatically on a disabled element. Therefore unfortunately what you suggested is not possible to achieve.
| if (firstErrorField) { | ||
| firstErrorField.focus(); | ||
| setTimeout(() => firstErrorField.focus(), 500); |
There was a problem hiding this comment.
what's the purpose of the half-second timeout?
There was a problem hiding this comment.
I cannot recall the reason for it now. However, I have removed it and looks it works okay :-).
| <Input | ||
| type="text" | ||
| id="email" | ||
| disabled={!isSendNewsletter} |
There was a problem hiding this comment.
Similar comment as before, this only applies if we're exposing these scenarios on our docs site for authors:
as a general usability best practice, I think it'd be better to show/hide this field based on !isSendNewsletter rather than toggling disabled state.
There was a problem hiding this comment.
I agree in the principle. Though as the purpose of these scenarios is at the first place to present to the testers all the different variants and types of the components in question, I have been a bit struggling to find a meaningful example of all these variants and types. Please, do you have a better idea for a disabled input to demonstrate in this scenario?
There was a problem hiding this comment.
I think the current use case is good for testing, and it's probably better to leave them as-is for now and run tests to get feedback sooner than later. Then maybe we can revisit this if we want to repurpose the examples for showing authors.
There was a problem hiding this comment.
Okay, anyway, do you please have a good idea for a disabled input example? It does not necessary hav to be part of this scenario. I could create a new scenario for that.
packages/react-components/react-components/src/AccessibilityScenarios/Textarea.stories.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const alert = React.useCallback(() => { | ||
| setIsAlerting(false); | ||
| setTimeout(() => setIsAlerting(true), 200); |
There was a problem hiding this comment.
This timeout and focus behavior causes focus to get sent back to the first field in the form that has an error, even if someone is working on an entirely different field and tabs away. In more detail: if I leave the first field in an error state to come back to later and move on, tabbing away from e.g. the 3rd field and triggering validation there will randomly send focus back to the first field.
I believe a better approach would be to only shift focus when a user activates the submit button.
There was a problem hiding this comment.
The focus should indeed move only when activating the "Send" button. When I tab through edit fields, the focus does not move to the first error field in my case. Could you please provide exact repro steps?
There was a problem hiding this comment.
First submit the form once without making any changes. Next navigate to the second textarea, and add a few characters (so the error message changes), and tab away. Your focus is then programmatically sent to the first textarea after a half-second delay.
The bug looks like it's in the useEffect on line 119, since after the form is submitted once, it runs again every time errors changes.
This same bug exists on the Input story.
There was a problem hiding this comment.
Thanks for the cue. Looks like I have fixed the bug, but I had to reintroduce the half second timeout for focusing the first error field upon form submission. Without the timeout, the focus sometimes stood at the submit button after submission.
Co-authored-by: Micah Godbolt <micahgodbolt@gmail.com>
* master: (106 commits) fix: PopoverTriggerChildProps should be exported (microsoft#25159) feat: replace ToolbarRadio implementation by usage of toggle button as Radio (microsoft#25343) docs: improve Toolbar docs examples (microsoft#25269) feat(tools): add unstable API setup updates (microsoft#25355) applying package updates Fix wrong narration when legend selected (microsoft#24903) applying package updates chore(react-persona): Update beachball settings and change file's type (microsoft#25363) chore: Refactor Field VR tests to have individual tests per component (microsoft#25263) chore(react-persona, react-components, vr-tests-v9): Reverting react-persona's version to beta (microsoft#25357) Publishing migration package (microsoft#25354) fix: Detailslist is still tabbable when isHeaderVisible=false (microsoft#25342) fix: list even/odd off-by-one issue (microsoft#25358) feat: add Dropdown a11y spec (microsoft#24917) spinbutton: update internal padding for small size (microsoft#25286) chore(global-context): migrate to new package structure (microsoft#25341) feat: Add validationState to Progress, to make the bar red or green (microsoft#25253) feat: Add accessibility scenarios for Fluent UI v9 components #3 (microsoft#23334) feat(Dropdown): Freeform search should be case insensitive (microsoft#24879) feat(what-input): Limit keyboard detection in inputs (microsoft#25087) ...
…ft#3 (microsoft#23334) * Add accessibility scenarios for the Accordion, Button and Popover components * Change scenario titles * Use string directly as component prop value Co-authored-by: Peter Varholak <peter.varholak@gmail.com> * replace alert with status text and fix warnings * Replace useCallback with ordinary function * Revert the title for the scenarios list page * Removed unnecessary fragment * Use <Label> components * Remove not used exports * Change files * Replace Calendar buttons scenario with Messenger buttons scenario, add Tooltip scenario and correct some wording * Add ToggleButton component scenario and replace button element with Button component in Tooltip scenario * Replace calendar scenario link with messenger scenario link and grammar changes * Add Link component scenario, and small changes in Accordion * Add beta version of SplitButton scenario * Make SplitButton scenario working * Change label for menu button within SplitButton * Replace Button with MenuButton in Menu scenario * Fix error in Menu scenario * Refactor aria-label for menu button within split button * Extract Postpone menu item into separate button * Add tooltip button without triggerAriaAttribute prop * Add other gender radio option * Rename disabled links and add nav region * Add heading and description for toggle buttons * Add FAQ accordion scenario and change radios in personal form accordion scenario * Add h1 for accordion scenarios * Fix lint errors and change Tooltip scenario for more real life examples * Add icon buttons for Tooltip scenario * Add x of Y and Submit button to simple form accordion scenario * Add menu with split item scenario * Change menu items in menu with split item scenario * Fix visible label in split button in menu secondary action * Remove X of Y in Accordion scenario * Enable dismiss of tooltip with Escape * Move password hint button between password label and input field * Reintroduce X of Y for personal form accordion scenario * Add as='h2' for accordion headers * Remove X of Y in personal form accordion scenario * Fix Tooltip error regarding relationship attribute * Add uncomplete Input scenario * Add slider component scenario and fix TS error in Input scenario * Change files * Fix import errors * Add screen reader supported validation to input scenario form * Add lint disable next line for complex object * Rename labels in slider scenario * Remove errorneously inserted unused declaration and import * Replace <input> with <Input> and add <form> * Remove already not needed import error ignore comments * Add onBlur validation for Input scenario and changed input requirements and hints * Replace 'any' with new type * Fix error messages flickering * Add aria=invalid in Input scenario * Fix nickname typo * Replace h1 heading with h2 * Implement form validation using React Hook Form * Ignore line max length lint error and add react-hook-form dependency * Focus error element upon form submission and narrate when already focused * Change form revalidate to onBlur * Replace custom validation with React Hook Form library, but narration is not working properly * toggle alert role to force re-reading of errors * fix narration on submit * Fix onblur error narration instead of first error narration on submit * Remove pubsub dependancy which was moved to root * Remove the password hint scenario for Tooltip component * fix lint issues * fix package * fix * cleanup * Replace native checkbox with Checkbox Fluent UI component * Fix typo in word 'reminder' * Add basic Checkbox scenario and consistency fixes in other scenarios * Add other accessibility relevant variants of Checkbox * Add temporary version of RadioGroup scenario and replace native HTML elements with Fluent UI components in some other scenarios * Replace native HTML checkbox elements with Fluent UI components * Replace native HTML input with Fluent UI Input component * Add disabled items variant for RadioGroup and some nit picking fixes in other scenarios * Add disabled RadioGroup variant * Replace tabindex values from 0 to -1 * Add Spinner, Switch and TabList components scenarios * Add disabled tab for TabList scenario * Add Notifications tab for TabList scenario * Add Textarea scenarioand changes to TabList scenario * Make checkboxes appear on a new line * Make textareas appear on a new line * Add vertical and overflow variants of TabList scenarios and small refactoring * Fix broken page with scenarios * Add Input with placeholder attribute * Add disabled Input example and preparation for readonly Input * Add lint ignore for long regex lines * Make <input> readonly and small refactoring of security code generating function * Proper setting of readOnly prpprop on Input component scenario * Add validation for Textarea scenario, add more variants and fix Input validation behavior * include accessibility scenario stories in the public docs * fix imports * fix imports * chore: add index.stories.tsx * Add FAQ accordion story * Add first half of v9 accessibility scenarios * Add remaining v9 accessibility scenarios * Make Send button not disabledFocusable * Make error message alerting more robust and remove 500ms setTimeout for first error field focus * Make overflow button in TabList not selected * Fix email validating birthDay instead * Fix email validation and some code cleaning * Fix first error field focus upon form submission problem * Make Input and Textarea scenarios more like a real example * Fix email validation when not required * Fix Input and Textarea scenario form validation and text changes * Fix invalid disabled textarea bug * fix scenarios link, remove them from sidebar again * Update apps/public-docsite-v9/.storybook/main.utils.js Co-authored-by: Micah Godbolt <micahgodbolt@gmail.com> * fix md file change * temporarily add `as` to MenuItem usage Co-authored-by: Peter Varholak <peter.varholak@gmail.com> Co-authored-by: Juraj Kapsiar <jukapsia@microsoft.com> Co-authored-by: Adam Samec <asamec@microsoft.com> Co-authored-by: Bernardo Sunderhus <bernardo.sunderhus@gmail.com> Co-authored-by: Juraj Kapsiar <jurokapsiar@gmail.com> Co-authored-by: Micah Godbolt <micahgodbolt@gmail.com>
Description of changes
This PR adds accessibility scenarios for testing of the Checkbox, Input, RadioGroup, Spinner, Switch, TabList and Textarea Fluent UI v9 components using a screen reader, and modifies the scenarios for some other existing components mainly so that they use Fluent UI components instead of native HTML elements.
The list of scenarios for testing can be found here.