Skip to content
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

Merged
merged 2 commits into from Oct 30, 2019

Conversation

@mkaz
Copy link
Member

mkaz commented Oct 29, 2019

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

Screenshot from 2019-10-29 15-24-18

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:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
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
@gziolo
gziolo approved these changes Oct 30, 2019
Copy link
Member

gziolo left a comment

Nice fix, thank you. Let's make sure this change is included in the docs before merging.

@mkaz mkaz merged commit 96f7d34 into WordPress:master Oct 30, 2019
7 checks passed
7 checks passed
pull-request-automation
Details
Header rules - gutenberg-playground No header rules processed
Details
Pages changed - gutenberg-playground All files already uploaded
Details
Redirect rules - gutenberg-playground No redirect rules processed
Details
Mixed content - gutenberg-playground No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
netlify/gutenberg-playground/deploy-preview Deploy preview ready!
Details
@mkaz mkaz deleted the mkaz:update/4726-color-palette branch Oct 30, 2019
@@ -55,7 +56,7 @@ export default function ColorPalette( {
<ColorPicker
color={ value }
onChangeComplete={ ( color ) => onChange( color.hex ) }

This comment has been minimized.

Copy link
@youknowriad

youknowriad Oct 31, 2019

Contributor

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.

This comment has been minimized.

Copy link
@gziolo

gziolo Oct 31, 2019

Member

Should this PR be reverted?

This comment has been minimized.

Copy link
@mkaz

mkaz Oct 31, 2019

Author Member

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

mkaz added a commit that referenced this pull request Oct 31, 2019
@youknowriad youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019
youknowriad added a commit that referenced this pull request Nov 11, 2019
CreativeDive added a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
…ress#18175)

* Add disableAlpha parameter to ColorPalette

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 WordPress#4726

* Update documentation with disableAlpha parameter
CreativeDive added a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.