-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix Star rating and thumb rating focus resolved by keyboard navigation #7466
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
Conversation
@@ -3,11 +3,14 @@ import { Star } from 'grommet-icons/icons/Star'; | |||
import { StarOutline } from 'grommet-icons/icons/StarOutline'; | |||
import { FormContext } from '../Form/FormContext'; | |||
import { RadioButtonGroup } from '../RadioButtonGroup'; | |||
import { StyledThumbsRatingBox } from '../ThumbsRating/ThumbsRating'; |
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.
can we also make StarRating its own box instead of importing StyledThumbsRatingBox
maybe StyledStarRatingBox
that way incase we have styles that are specific to either component they are mutually exclusive.
@umeshiscreative |
91df026
to
01dddf0
Compare
@umeshiscreative code looks good trying to see why the there was a snap shot change though |
src/js/components/Accordion/__tests__/__snapshots__/Accordion-test.tsx.snap
Outdated
Show resolved
Hide resolved
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.
Looks good thank you for your contributions.
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.
Looks good!
What does this PR do?
Fix Star rating and thumb rating focus resolved by keyboard navigation
Where should the reviewer start?
components/ThumbStarRating.js
components/StarRating.js
What testing has been done on this PR?
Updated Snapshot for this fix
How should this be manually tested?
Tested in Chromium based browser with storybook
Do Jest tests follow these best practices?
screen
is used for querying.asFragment()
is used for snapshot testing.Any background context you want to provide?
Wrapping the StarRating and thumb rating component with StyledThumbratingBox
What are the relevant issues?
#7452 #7453
Screenshots (if appropriate)
Do the grommet docs need to be updated?
No
Should this PR be mentioned in the release notes?
No
Is this change backwards compatible or is it a breaking change?
Not Sure