-
Notifications
You must be signed in to change notification settings - Fork 359
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
Docs: upgrade to Create React App 4 #1276
Conversation
Deploy preview for gestalt ready! Built with commit 7e06bb0 |
d275bba
to
7c8c20f
Compare
7c8c20f
to
7e06bb0
Compare
@@ -55,8 +55,8 @@ describe('Button', () => { | |||
render(<Button text="test" disabled ref={ref} />); | |||
expect(ref.current instanceof HTMLButtonElement).toEqual(true); | |||
expect( | |||
ref.current instanceof HTMLButtonElement && ref.current?.tabIndex |
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.
@AlbertCarreras could you verify that this is correct?
When I looked at a disabled <Button />
, we only set the disabled
attribute, we do not set the tabIndex
:
- Go to https://gestalt.netlify.app/Button
- Toggle
Disable buttons
- Inspect the HTML in the Chrome devtools
- No
tabIndex
is set on the button
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.
</Switch> | ||
</App> | ||
</BrowserRouter> | ||
<AppWrapper /> |
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.
@ayeshakmaz @AlbertCarreras fyi had to move this wrapper outside of index.js
since otherwise fast refresh does not detect incremental changes to the docs
https://github.com/facebook/create-react-app/blob/master/CHANGELOG.md#400-2020-10-23
Test Plan
yarn start