Skip to content

Conversation

miqh
Copy link
Contributor

@miqh miqh commented Oct 14, 2022

Hello there,

This should fix up the accessibility violations reported across multiple RangeInput component stories.

As an aside, I noticed a11yTitle feeds into aria-label of the underlying styled range input. Having looked at FileInput just the other day, the latter doesn't appear to do the same, so this might be a potential inconsistency worth reviewing?

In any case, there's no reason why we can't also fix the violations here by directly specifying aria-label instead of a11yTitle because the RangeInput appears to feed everything through as spread props.


What does this PR do?

Resolves an accessibility violations for the RangeInput component.

Where should the reviewer start?

Across the various stories of the RangeInput component.

What testing has been done on this PR?

Confirmed the violations no longer appear in Storybook.

How should this be manually tested?

As above.

Do Jest tests follow these best practices?

N/A.

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

None.

What are the relevant issues?

Refer to #6124.

Screenshots (if appropriate)

N/A.

Do the grommet docs need to be updated?

Don't think so.

Should this PR be mentioned in the release notes?

Up to maintainers to decide.

Is this change backwards compatible or is it a breaking change?

Should have no impact on consumers of the library.

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@ericsoderberghp ericsoderberghp merged commit 40ce986 into grommet:master Oct 17, 2022
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.

3 participants