-
Notifications
You must be signed in to change notification settings - Fork 1
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
A11Y Improvements -- Render buttons as buttons #154
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this change! I just had one minor comment to address styling that will likely carry forward into future a11y changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just noticed one more minor thing!
We should be passing disabled={disabled}
to StyledButton
so that form submissions with the button are not possible if we've disabled the button. Minor thing, but it currently we can use Generate Book button even if it's disabled!
No problem, but I want to make sure I am looking at the right thing. In |
Sorry, the GitHub reviewer doesn't let you make comments on any line in the code yet. Yep, the change should be made in |
I think this is good to go now. I changed it to I also noticed in testing that the button maintained the click cursor if the button has been disabled, so I fixed that also. |
Looking good, these should be the last few comments! |
I think we are good to go now, sorry you've had to go over it so many times! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks great now!
Per #148, I have been playing with the application tonight and the first thing that leapt out to me was that the form on the "New Project" modal was not able to be submitted by keyboard. The root cause of this that this PR fixes is rendering buttons as buttons rather than a div/span.
This can cause multiple accessibility challenges both to keyboard navigation and control and also screen reader identification. Forms throughout the application are now able to be submitted by keyboard so long as they utilize a real
<form>
element and anonSubmit
property.