-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
refactor: Rewrites ColorSchemeControl with Typescript #21496
refactor: Rewrites ColorSchemeControl with Typescript #21496
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21496 +/- ##
==========================================
- Coverage 66.66% 66.66% -0.01%
==========================================
Files 1793 1793
Lines 68492 68494 +2
Branches 7277 7282 +5
==========================================
- Hits 45663 45660 -3
- Misses 20967 20972 +5
Partials 1862 1862
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
/testenv up |
@geido Container image not yet published for this PR. Please try again when build is complete. |
@geido Ephemeral environment creation failed. Please check the Actions logs for details. |
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.
Code LGTM. Would be nice to have QA approval
@jinghua-qa Can you fetch this branch locally and test it? It seems ephemeral environments are failing for this PR. |
I have tested this pr locally, worked as expected |
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
SUMMARY
This PR rewrites the
ColorSchemeControl
component using Typescript and a functional component. The main reason for refactoring the component was that the previous version was re-rendering theSelect
component many times by mutating theoptions
property when the user hovered the elements. This caused a bug in reordering the selected items when hovering in/out.TESTING INSTRUCTIONS
We shouldn't have any function changes and the selected items should remain on top when hovering in/out.
ADDITIONAL INFORMATION