-
Notifications
You must be signed in to change notification settings - Fork 30
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
fix(Button): remove button as default role #2876
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.
lgtm! no longer seeing role="button"
for the buttons rendered in SB :)
I had some questions about the test, but more so to learn and not blockers.
|
||
describe('when the button is disabled', () => { | ||
it('does not render the href attribute and renders a button', () => { | ||
const buttonText = 'Submit'; |
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.
does this need to be set again? doesn't it have access to buttonText
in line 5?
}); | ||
|
||
it('renders the disabled attribute on button tags', () => { | ||
const buttonText = 'Submit'; |
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.
same Q here about setting buttonText
📬Published Alpha Packages:@codecademy/[email protected] |
🚀 Styleguide deploy preview ready! |
Overview
Remove unnecessary button default role
Pointed out here
PR Checklist
Testing Instructions
PR Links and Envs