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

Add support for hexadecimal colors in gradient presets #23363

Merged
merged 1 commit into from
Jun 24, 2020
Merged

Add support for hexadecimal colors in gradient presets #23363

merged 1 commit into from
Jun 24, 2020

Conversation

properlypurple
Copy link
Contributor

@properlypurple properlypurple commented Jun 22, 2020

Description

This PR makes sure that the custom gradient picker is able to parse colors in the hexadecimal format.

How has this been tested?

  1. Added this code to theme
add_theme_support( 
  'editor-gradient-presets', 
  array(
    array(
      'name'     => __( 'Hex gradient', 'themeslug'  ),
      'gradient' => 'linear-gradient(to bottom, #000000 50%, #123456 100%)',
      'slug'     => 'hex-gradient'
    )
  )
);

  1. Set above described gradient background for a group block.
  2. Success

This fixes the bug #23361

Types of changes

Bug fix

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.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works on my end. 👍

If we can get those tests to pass I think it's good to go.

Before:

hex-color-old

After:

hex-color-new

@jffng
Copy link
Contributor

jffng commented Jun 23, 2020

cc @jorgefilipecosta as the author of #22239.

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have verified that this fixes the issue.

There is still some strangeness between the custom gradient control point picker when one of these presets is selected, but it seems unrelated:

Kapture 2020-06-23 at 14 25 40

@kjellr
Copy link
Contributor

kjellr commented Jun 23, 2020

There is still some strangeness between the custom gradient control point picker when one of these presets is selected, but it seems unrelated

Hmm, interesting — I hadn't seen that. @youknowriad can you provide a gut check on that too (or do you know who else might able to since Jorge is away at the moment)?

@youknowriad
Copy link
Contributor

I think this is probably just half of the story :)

While now they don't break which is cool, editing them is not possible. As soon as you touch the custom picker, it resets the gradient. Not sure yet what's missing there though.

@kjellr
Copy link
Contributor

kjellr commented Jun 24, 2020

😄 Ok. Do you think this is ok to merge in the meantime then? And we can file another issue for the custom picker?

@youknowriad
Copy link
Contributor

Yes, but let's file an issue to track this :)

@properlypurple
Copy link
Contributor Author

properlypurple commented Jun 24, 2020

I have done some more testing, and it looks like there are a few issues here

  1. The control doesn't normalize the gradient direction to horizontal, when in the slider
  2. Colors for angular gradients aren't picked up properly by the control even when the colors are rgb() or rgba().
  3. Gradients need explicit stops, so something like this won't work

'gradient' => 'linear-gradient(to top left, ' . $gradient_color_a . ', ' . $gradient_color_b . ' )',

I'm doing some more testing to see if i can find more edge cases, and I'll file individual bugs for these.

@kjellr
Copy link
Contributor

kjellr commented Jun 24, 2020

Thank you, @properlypurple! I'll merge this in the meantime. Thanks for the PR and for all the investigation!

@kjellr kjellr merged commit a81d2f5 into WordPress:master Jun 24, 2020
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jun 24, 2020
@github-actions
Copy link

Congratulations on your first merged pull request, @properlypurple! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@jorgefilipecosta
Copy link
Member

😄 Ok. Do you think this is ok to merge in the meantime then? And we can file another issue for the custom picker?

Hi @kjellr, I guess I can try to work on this issue if it still happens. Was an issue created so I can reference it and check the status of the task or we did not create and in that case I guess I can directly open a PR?

@kjellr
Copy link
Contributor

kjellr commented Aug 10, 2020

Was an issue created so I can reference it and check the status of the task or we did not create and in that case I guess I can directly open a PR?

Hi @jorgefilipecosta! Yes, @properlypurple created followup issues in #23501 and #23503.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants