Conversation
|
I hadn't noticed this ticket before and stumbled onto the same automated testing finding, proposing some similar changes at #7484. I'm happy to close my pull request and/or consolidate some of the changes. I like the idea of the server generating an ID for the For the changes which effectively hide the description by changing the |
…nly includes ids for errors and hints that exist
svalexander
changed the title
@aduth No specific reason, had started with the show function first and so just did the opposite of that with hide to ensure if a user corrected the error the aria-describedby wouldn't include the error id. I was going to close out my pr since yours covered the same issue. I think it would be useful to link to the Lg-7579 ticket in yours so we can track the pr. |
|
Sure, happy to continue and collaborate in #7484. I'll add a link to the ticket there. I also like the idea you implemented here with the I think we may have been hiding the element since |
Inspired by @svalexander's idea in #7488 Co-Authored-By: Shannon A <20867088+svalexander@users.noreply.github.com>
) * Avoid ValidatedFieldComponent missing aria-describedby references changelog: Bug Fixes, Accessibility, Fix missing ID references for field descriptors * Fix specs * Remove unnecessary non-null operator * Add error-id as attribute of lg-validated-field Inspired by @svalexander's idea in #7488 Co-Authored-By: Shannon A <20867088+svalexander@users.noreply.github.com> * Try avoiding layout-affecting padding for empty error message May want to revisit. Could also consider ":empty" pseudo-selector * Make spec descriptor hint more realistic Avoid describedby associated to a label (which would be labelledby) * Sync MemorableDateComponent markup See: #7484 (comment) * Revert "Try avoiding layout-affecting padding for empty error message" This reverts commit d12c08a. * Update MemorableDate component to add error-id reference * Create helper methods to consolidate logic for hint/error ID * Restore display: none styling for valid field * Add describedby association as part of input validity handling * Remove accessible description assertion Previously, I picked "month" as an arbitrary field to find the error, but since now the field validity aria-describedby is managed per-field, the relationship would happen at the field level. We can (and ideally should) check the accessible description for the field we're interested in checking. * Avoid excess spaces in aria-describedby technically not an issue, but we should avoid having "empty" id refs be considered * Assert error message as hidden we could revert to what existed previously with "not_to have_css", but stronger guarantee that we check both the text is empty, and that it's explicitly present but not visible * Update MemorableDateComponent specs * Remove debugging code Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com>
2 participants
🎫 Ticket
Lg-7579
🛠 Summary of changes
This pr addresses two issues with the ValidatedFieldComponent. Currently when the component is first rendered the aria-describedby attribute includes "validated-field-hint-{id}" and "validated-field-error-{id}". The hint id is included even if there is not a hint, and the error id is included before an actual hint exists. These id's should not be included in aria-describedby unless the hint and error exist.
To correct the hint issue, a check is added in the validated-field-component.html.erb for the presence of the hint.
For the error issue, the error id is shown when the input is invalid (user has submitted field w/o valid input), and also adds the error id to the element that displays the error (this is to create a connection between the error element and the validated field component).
📜 Testing Plan
Provide a checklist of steps to confirm the changes.