-
Notifications
You must be signed in to change notification settings - Fork 4.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
Update ColorPalette component to accept disableAlpha parameter #18175
Conversation
Adds the disableAlpha parameter to ColorPalette component which is passed forward to the ColorPicker to show. Previously it was set to true by default. Fixes #4726
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.
Nice fix, thank you. Let's make sure this change is included in the docs before merging.
@@ -55,7 +56,7 @@ export default function ColorPalette( { | |||
<ColorPicker | |||
color={ value } | |||
onChangeComplete={ ( color ) => onChange( color.hex ) } |
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 far as I know hex colors format don't include "alpha" information in it. This make me think this PR is not complete yet as there's no way for the users of this component to get the value of the selected alpha.
Should we switch to rgba if disableAlpha is false? should we do it every time? do we need a format prop? should we pass the whole color object. These are all options.
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.
Should this PR be reverted?
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.
ah... that might explain why it was disabled in the first place.
I think we should revert and then we can add to address other issues.
I prepped this PR for the revert: #18220
Description
Adds the disableAlpha parameter to ColorPalette component which is passed forward to the ColorPicker to show. Previously it was set to true by default.
Fixes #4726
How has this been tested?
Run
npm run design-system:dev
to view component in Storybook.Click custom color and confirm the Alpha channel shows up.
Screenshots
Types of changes
Update ColorPalette component previously hard-coded to false when using the ColorPicker, this allows the parameter to be set by user.
Checklist: