Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add label and aria-expanded for Javalab file options toggle button #49473

Merged
merged 4 commits into from
Jan 5, 2023

Conversation

rshipp
Copy link
Contributor

@rshipp rshipp commented Dec 13, 2022

Add a label to the file explorer button in Javalab. I used what looked like the simplest approach in this article on accessible icon buttons, and a label text that matches the behavior of the button. Let me know if you think I should change any of that!

image

Links

@rshipp rshipp requested a review from a team December 13, 2022 18:36
@@ -714,6 +714,7 @@ class JavalabEditor extends React.Component {
<span>{fileMetadata[tabKey]}</span>
{activeTabKey === tabKey && !isReadOnlyWorkspace && (
<button
aria-label="File options"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this string should be translatable

@rshipp rshipp requested a review from a team as a code owner December 13, 2022 20:34
@rshipp rshipp changed the title Add label for Javalab file options toggle button Add label and aria-expanded for Javalab file options toggle button Dec 14, 2022
@rshipp rshipp requested a review from a team December 14, 2022 22:21
@@ -714,6 +714,8 @@ class JavalabEditor extends React.Component {
<span>{fileMetadata[tabKey]}</span>
{activeTabKey === tabKey && !isReadOnlyWorkspace && (
<button
aria-label={i18n.fileOptions()}
aria-expanded={contextTarget === tabKey ? true : false}
Copy link
Contributor Author

@rshipp rshipp Dec 14, 2022

Choose a reason for hiding this comment

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

(this logic is copied from ~10 lines below). i could also simplify it to contextTarget === tabKey, i'm not sure which is more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think contextTarget === tabKey is more readable

@rshipp rshipp merged commit 0dff88d into staging Jan 5, 2023
@rshipp rshipp deleted the a11y-13 branch January 5, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants