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

Increase Accessibility #904 #910

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Mish2j
Copy link

@Mish2j Mish2j commented Apr 25, 2023

what needs to be done:

  1. add names to buttons
  2. form elements do not have labels
  3. links do not have names

1 and 3 have been completed. For 2 there is no elegant way to add labels on form elements, since the form element (textarea) is a child element of the "LiveEditor" component imported from "react-live-runner".

@@ -150,9 +150,10 @@ class ModalContainer extends PureComponent {
}
}

const LogoLink = styled(Link).attrs((/* props */) => ({
const LogoLink = styled(Link).attrs(({ ariaLabel }) => ({
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 you can just pass aria-label directly on LogoLink instead of using the attrs indirection:

<LogoLink aria-label="Styled Components Home Page">

unstyled: true,
'aria-label': props.ariaLabel,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@quantizor
Copy link
Contributor

This is looking pretty good to me, the only comment I have is we might want to just use title instead of aria-label so you get the nice hovertext as well for non-screen readers

@Mish2j
Copy link
Author

Mish2j commented May 4, 2023

This is looking pretty good to me, the only comment I have is we might want to just use title instead of aria-label so you get the nice hovertext as well for non-screen readers

Hi @probablyup, thanks for the feedback. I'll work on this in the next few days...
Regarding the title attribute, I think it would be better to have both title and aria-label since title is not always reliable as stated on W3.org

The title attribute can also be used to identify form controls. This approach is generally less reliable and not recommended because some screen readers and assistive technologies do not interpret the title attribute as a replacement for the label element, possibly because the title attribute is often used to provide non-essential information. The information of the title attribute is shown to visual users as a tool tip when hovering over the form field with the mouse.

Let me know what you think...

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.

None yet

2 participants