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

Accessibility improvements (and a few small other things) #1

Merged
merged 7 commits into from
Oct 9, 2021

Conversation

moritzjacobs
Copy link
Contributor

Hi! Great component, I want to use it in our project, but we have more strict a11y checks in place, so I did the following changes:

  • Chrome showed me this warning in the console, because the ARIA role parameter was incorrect
  • I added keyboard accessibility, so you can toggle spoilers with the enter key and space bar
  • the lint tests weren't working so I fixed the command in test:lint
  • to get the project running I had to change some config
  • I changed the default colors for the spoiler to something that will more easily adapt to all sorts of circumstances
  • CRA automatically changed the tsconfig and this react-app-env.d.ts file, I'm not sure why but it probably can't hurt

Maybe it will help some other people out. Thank you for your consideration and greetings from Augsburg!

Comment on lines +16 to +17
hiddenColor = 'currentColor',
revealedColor = 'transparent',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

currentColor will always be the text color and I would guess in 99% of cases, you would want your text to be "regular" after the reveal.

@moritzjacobs
Copy link
Contributor Author

Hi @dazulu — is there any activity on this project? :)

@dazulu
Copy link
Owner

dazulu commented Sep 2, 2021

Hi @dazulu — is there any activity on this project? :)

Slipped through the cracks, sorry! Will take a look at your PR.

@dazulu dazulu merged commit 4c8b0b6 into dazulu:master Oct 9, 2021
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