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
Removing Material from slider #1244
Conversation
…eact into jialecl-removingMaterial-slider
…eact into jialecl-removingMaterial-slider
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.
Other changes
-
I think we should be consistent in how we implement all the components. In your case you are setting the type next to the styled component, I propose you create types for each one naming them as
XXXXPropsType
. -
Add new tests:
- Test case for all accessibility attributes.
- Test cases for all keyboard interactions.
- Test case we saw in the call:
minValue={5} maxValue={50} step={10}
. Verify that the maximum value is 45.
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.
- As I said in the previous comment:
- Add test case for ALL accessibility attributes, including:
aria-labelledby
,aria-orientation
,aria-valuemax
,aria-valuemin
,aria-valuenow
. This should be added to the first test case, not others. If you need an example, take a look to the first test case from Radio Group component. - The disabled tracker changes its styles when clicked.
- Add test case for ALL accessibility attributes, including:
No description provided.