Skip to content

LG-7766: Update hint text and label for memorable date component#7365

Merged
svalexander merged 10 commits intomainfrom
shannon/lg-7766-memorable-date-hint-and-label
Dec 1, 2022
Merged

LG-7766: Update hint text and label for memorable date component#7365
svalexander merged 10 commits intomainfrom
shannon/lg-7766-memorable-date-hint-and-label

Conversation

@svalexander
Copy link
Contributor

🎫 Ticket

Lg-7766

🛠 Summary of changes

This pr updates the hint text for the memorable date component. It updates the hint text for each of the inputs (month, day and year) by removing the reference to the hint under the label. It replaces that hint with more specific hints for each input. Also updated the label for the component so it matches other components on the form page.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Navigate to the form page
  • Use a screen reader to navigate through the page
  • Verify hint under date of birth label is read
  • Verify hint text for month is "example: 4"
  • Verify hint text for day is "example: 28"
  • Verify hint text for year is "example: 1986"

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

It would be good if we could have some spec coverage for this. The Capybara finders should be able to locate a field by its accessible computed name which would be useful to be able to find the field with the expected label "Month Example: 4", though I'm not sure how well it works with multiple distinct labels for the same field like what we're implementing here.

"validated-field-hint-#{unique_id}",
],
describedby: "validated-field-error-#{unique_id}",
labelledby: 'month_hint_id',
Copy link
Contributor

Choose a reason for hiding this comment

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

A page can potentially have more than one instance of this component, so I think we should use unique_id like with the describedby to avoid any risk of duplicate IDs.

Suggested change
labelledby: 'month_hint_id',
labelledby: "memorable-date-month-hint-#{unique_id}",

Copy link
Contributor

Choose a reason for hiding this comment

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

Would assigning aria-labelledby to just the hint risk that the user would not be informed that this is the "Month" field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it makes sense to add the id for sure. I did add it in but having trouble with the span - using the inspect tool I see the id in the span still says "unique_id" rather than the id that should have been generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait, i got it working now. I was missing this syntax: <%={id} %>

@svalexander svalexander requested a review from aduth November 22, 2022 16:19
"validated-field-hint-#{unique_id}",
],
describedby: "validated-field-error-#{unique_id}",
labelledby: "memorable-date-month-hint-#{unique_id}",
Copy link
Contributor

Choose a reason for hiding this comment

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

I mentioned at #7365 (comment), but I might worry how a screen reader would handle the fact that we use both <label for="..."> + aria-labelledby to label each field. Maybe for extra assurance, do you think it'd help to create an ID for the label and then set aria-labelledby to point to both the label element and the hint text?

Suggested change
labelledby: "memorable-date-month-hint-#{unique_id}",
labelledby: [
"memorable-date-month-label-#{unique_id}",
"memorable-date-month-hint-#{unique_id}",
],

Copy link
Contributor

Choose a reason for hiding this comment

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

Testing this in Safari + VoiceOver, the "Month" label is currently not read when focusing the field.

month-field.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh i think i misunderstood the comment the 1st time, updated so it has the label id. that was definitely keeping the label from being associated with the input field.

@aduth
Copy link
Contributor

aduth commented Nov 22, 2022

Can we add a spec which tests that the hint is included in the field label?

@tomas-nava tomas-nava changed the title Shannon/lg 7766 memorable date hint and label LG-7766: Update hint text and label for memorable date component Nov 29, 2022
@svalexander svalexander requested a review from aduth November 30, 2022 14:49
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@svalexander svalexander merged commit 3c69b31 into main Dec 1, 2022
@svalexander svalexander deleted the shannon/lg-7766-memorable-date-hint-and-label branch December 1, 2022 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants