Skip to content

changelog: Content Change, Update screen reader message to identify a…#6044

Merged
SammySteiner merged 5 commits intomainfrom
LG-5857-accessibility-ism-read-buttons
Mar 11, 2022
Merged

changelog: Content Change, Update screen reader message to identify a…#6044
SammySteiner merged 5 commits intomainfrom
LG-5857-accessibility-ism-read-buttons

Conversation

@SammySteiner
Copy link
Contributor

…ll options, LG-5857

@SammySteiner SammySteiner force-pushed the LG-5857-accessibility-ism-read-buttons branch from b073f42 to eaa38a5 Compare March 11, 2022 16:06
@SammySteiner SammySteiner force-pushed the LG-5857-accessibility-ism-read-buttons branch from eaa38a5 to 14f376d Compare March 11, 2022 16:07
class: 'usa-button usa-button--big usa-button--full-width margin-bottom-2' %>
<%= link_to modal.sign_out,
destroy_user_session_path,
role: 'link',
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there any change in functionality after adding this role? link_to renders an anchor tag and would have the role explicitly stated on it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it got left in from when I was experimenting with turning it into a button tag.

<div class="usa-modal-overlay">
<%= tag.div(
role: 'dialog',
role: 'alertdialog',
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a little concerned about this because this layout is used on all the modals in the app. Is there a way we can use logic to determine if the role should be alertdialog vs dialog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, ok, I added locals parameter to the render and updated the three places where it is used.

@SammySteiner SammySteiner merged commit 147995f into main Mar 11, 2022
@SammySteiner SammySteiner deleted the LG-5857-accessibility-ism-read-buttons branch March 11, 2022 19:55
@@ -0,0 +1,248 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, apologies I didn't catch this, but this PR ended up checking in some generated HTML into source code, it's noisy, derived data and doesn't belong here.

I've got a commit to clean this up and prevent it in the future: 9603127

@aduth aduth mentioned this pull request Mar 15, 2022
@aduth
Copy link
Contributor

aduth commented Mar 15, 2022

These changes have the impact of causing the time to be announced every second. Is that expected? Might be caused by the change from role="dialog" to role="alertdialog".

Screen.Recording.2022-03-15.at.11.47.46.AM.mov

@SammySteiner
Copy link
Contributor Author

These changes have the impact of causing the time to be announced every second. Is that expected? Might be caused by the change from role="dialog" to role="alertdialog".

Screen.Recording.2022-03-15.at.11.47.46.AM.mov

So weird, I took a bunch of recordings of the behavior for the acceptance thread with my local deployment and it didn't behave this way.

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.

5 participants