Implement quick and dirty error message solution plus a few additional accessibility fixes #581
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
bacall213
commented
Apr 2, 2025
bacall213
commented
- Implement the quick and dirty solution from Toast duration field doesn't have a minimum or maximum value set #563
- Add a few minor CSS tweaks to keep text boxes from shuffling around when receiving and losing focus
- Add minimum value (500ms) to toast duration input field - Add "(opens in new window)" to links that open in new windows (https://www.w3.org/TR/WCAG20-TECHS/G201.html). - Move a period out of the link one of the opens in a new window links.
The border for focused boxes is 2px but the border for unfocused boxes is only 1px. By setting a 1px margin by default we create extra empty space and then change the margin back to zero when the box is focused so the 2px border takes up that space.
Setting a min value in NumberInput only changes the limits for the input field but won't prevent someone from entering a number less than the minimum. This forces values to be at least the minimum (500ms).
✅ Deploy Preview for voluble-crisp-6e2a75 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
I've been playing around with this a bit more and I have some additional improvements to add when it comes to contrast and focus. I also think I found a way to get the errors announced properly. Setting the alert div to role="alert" and aria-live="polite" is one part of it, and then passing in the box label to the error so the error message has context associated with it is the other part. The resulting readout to screen readers is is something like "Border radius (px) Please enter a value less than 72" |
|
I think for the external links we could use an icon and then have the text "Opens in new tab" as a .sr-only span after the icon. I would create a component to simplify this. import { faExternalLink } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
export function ExternalLink({
href,
children,
}: {
readonly href: string;
readonly children: React.ReactNode;
}) {
return (
<a href={href} target="_blank" rel="noreferrer">
{children} <FontAwesomeIcon icon={faExternalLink} aria-hidden="true" />
<span className="sr-only">Opens in new tab</span>
</a>
);
}You need to create a .sr-only class in index.css: .sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
white-space: nowrap;
border-width: 0;
}Then we can use it like this: <p className="small show-on-focus">
Use a{" "}
<ExternalLink href="https://developer.mozilla.org/en-US/docs/Web/CSS/color_value">
CSS color string
</ExternalLink>
.
</p>As for the other changes, you can add them to this PR since it's all related to accessibility or create a new one. Whatever you prefer. |
Replace open in new tab implementation with David's suggested solution for external links
1. Move focus back to the add button after deleting an exclude keys entry.
Focus is currently being returned to an invisible spot instead of the actual button. This change returns it to the add button every time.
If we return it to the delete button, if a row exists, then a user who clicks twice accidentally might delete an entry they wanted. Moving focus back to the Add button helps protect the user from making a change they can't undo.
useRef was added for this and then called in the handleDeletion() function: `addExcludeKeyButtonRef.current?.focus();`
2. Move the explanation text for exclude keys up so the explanation comes first.
3. Change the icon colors to darker colors that have better contrast for a white or black background. Also change the icon color references to use the variables used elsewhere in settings so they'll remain consistent.
4. Change the icons to the larger version. For the trash can, in particular, this makes the target more visible.
5. Add aria label "Add exclude keys pattern" to the add button so it's clear for screen reader users what the button does.
6. Add aria label to the keys to exclude text field so the purpose of the field is more apparent.
7. Add an aria label to the trashcan that makes it apparent what pattern will be deleted: `aria-label={"Delete pattern " + entry[0]}`
1. Use color variable references for the icons to keep things consistent. 2. Bump the size of some of the icons so the target is easier to hit. 3. Use `useRef` to assign a reference to the "Add selector to include" button so that we can return focus back to that button after a row is deleted. Similar to exclude keys, this is the safer option since there is no undo for deleting an entry.
1. Add an in-between slate color (slate-350) to use for the toggle backgrounds so a disabled toggle maintains at least 4.5:1 contrast ratio regardless of whether the page has a white or black background (in case you implement dark mode later). Since the toggles are relatively small and they're control elements, I choose a color that works with the WCAG AA text requirement instead of just the graphical element requirement. 2. Change green-500 and red-500 to darker variants that are more contrasting (meeting >= 4.5:1). The trashcan doesn't have any text label associated with it so the higher contrast is needed. For the toggles, the green will maintain >= 4.5:1 contrast for both a white and black background. 3. Add the screen reader only formatting for the "open in new tab" addition. 4. Change the button size to 1rem so the buttons match with the surrounding elements.
Wrap the alert text in a div so there's a spot in the DOM for the alert to appear instead of the entire thing needing to be created. It might be possible to make it work without this, but when I was testing the alerts with VoiceOver it seemed like I needed something already present that the aria-live component drops into for it to work correctly.
Call notifyTogglesStatus() after toggling the hints at a global level. This results in a status toast whether you click the icon or use the keyboard shortcut. For reasons I couldn't track down, the status toast was only being displayed when called with Talon directly. Displaying the status when the icon is clicked or activated will help users understand what happened if they perform the action accidentally.
Use a darker base color in the toast notifications.
Additional color adjustments for toast notifications. This also makes the close button more contrasty.
Increase the contrast of the "unset" indicators.
Minor color adjustments to improve contrast ratios. The green matches green-500 and the red matches red-500.
1. Pass the element ID into the alert so we can attach it to the same label as the input box that generated the error. 2. Assign the aria role="alert" and aria-live="polite" to announce the errors (politely, of course)
bacall213
changed the title
|
One of the lingering issues is that the toast notifications use an aria-hidden icon for reporting the state. This is mostly an issue for the list of hint statuses. The toast is set to role="alert" by default so it's being read by screen readers, but the actual useful values for the states aren't being read. I found a way to add aria-labels to them and reference the state variable, but since the toast doesn't receive focus (and I'm not sure that it should) the aria-labels aren't read. I'm not sure yet what the best way to solve this one is. |
Change link text so the purpose of the link is more clear.
Add a "main" landmark to the overall div and then move the footer out of it so there aren't nested landmarks. Adding a <main> tag wrapping <div class="container"> would have worked as well.
|
The toast notifications look like they would be considered a status message and shouldn't receive focus, but the underlying content still needs to be announced. Still trying to find a way to solve for that. After some more testing, the alert boxes don't need the extra div tags to work reliably so I'll remove them from the updated file in the pull request. |
Comment on lines
23
to
37
| function ExternalLink({ | ||
| href, | ||
| children, | ||
| }: { | ||
| readonly href: string; | ||
| readonly children: React.ReactNode; | ||
| }) { | ||
| return ( | ||
| <a href={href} target="_blank" rel="noreferrer"> | ||
| {children} <FontAwesomeIcon icon={faExternalLink} aria-hidden="true" /> | ||
| <span className="sr-only">Opens in new tab</span> | ||
| </a> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Better move this to its own file
- Assign the aria role based on the type passed into the alert. - Cleanup some of the extra pieces that didn't work as well.
Move ExternalLink function into separate file per comment on pull request.
- Add updated validation bits
- Update validation bits using the zod validation features in the settings schema
- Move ExternalLink bits out to separate file and import it - Add updated validation pieces Note on validation - I couldn't figure out which type was needed to handle an object and then reliably get the boolean for 'valid' out of it and the optional 'message' field without typescript throwing errors so I kept them separate as 'isValid' (still a boolean) and 'validationMessage' (string)
Additional changes to the button labeling for screen reader users.
Additional changes to button labeling for screen reader users.
Add the checkValidity function David mentioned earlier in the pull request discussion.
Update the settings schema with appropriate validation error messages for various fields.
Changes the title of the notifications to "Rango notifier:" for screen reader users. When screen readers read out the toast notification, the semantic relationships are lost and all of the text runs together. For the toggle status notification, the result is something like "Rango Now Page Host Global dismiss to close" This change, combined with the others, help make the readout make more sense for screen reader users without affecting the visual formatting.
Utilize the sr-only css class to add additional content to the readout of notifications for screen reader users. I initially tried to solve for this by adding labels to the icons and unhiding them from screen readers, but the toggle status notification puts the icons on the right and the label on the left whereas the other notifications put the icon on the left and the text on the right. The non-toggle notifications already included the status information in the accessible text. Adding the label to the image made them nonsensical. Instead, this adds additional helper text after the visual label that's only exposed to screen reader users. getIconType() returns the exact text needed, so I used that in the screen reader output. The expected result for a toggle status dialog is something like: "Rango notifier: Now hints are unset. Page hints are unset. host hints are disabled. Global hints are enabled. dismiss to close" For non-toggle notifications, the result is something like "Rango notifier: keyboard clicking enabled"
This provides additional positional information for screen reader users since the rows are not presented using standard table elements.
Specific to screen reader users since the content isn't rendered using table elements. There are no visible changes that will result from this.
Moved to settingsSchema
The alert dialog was a bit too much. This follows the same pattern in the other components where an explanation message is made visible when a field is disabled.
The regex and selector fields both performed validation, but didn't reset the value when the user leaves the field even if the value didn't parse correctly. The validation styling relied on color alone to alert the user to an issue.
This adds a separate block to display validation alerts for the custom hints.
I wanted to put the alerts immediately below the appropriate box or row, but I couldn't get it to work properly in a way that passed the typescript tests. If you know of a better way to add on the alerts, I'm open to alternatives.
This change also adds aria-invalid to the regex text fields, but not to the selector text fields. When adding it to the selector text field, it technically works (and works well), but it gets caught as having an invalid value.
`aria-invalid={selector && !isValidSelector(selector)}` is what didn't work.
A 'message' can be defined for some (all?) zod objects, including z.number(). By assigning an error message to the 'message' property for each schema item (where relevant), we negate the need for the extra logic in NumberInput.tsx to handle NaN instances. The value of 'message' will be returned instead. When the value is outside of the .refine() range, invalid input is returned. We can also apply the same message to that.
Instead writing the same message twice, errors = {...} was added at the top to store the various error messages.
There might be a better way to do this with a custom error map in zod, but this seems to work quite well.
The previous push added a second iterator which was probably not resource friendly. To get around that, I had to wrap the entire block in another div which then required some additional CSS changes to keep the formatting consistent. One benefit of this change is that now the alert boxes will appear immediately below the row where the error is occuring.
|
I added a commit. It has nothing to do with your changes. I just noticed it and since it's related I added it here. What's the status of this PR, are you going to make more changes or is it ready to merge? I want to make a release soon and I want to include it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.