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

Duotone Tracking & History #26361

Closed
wants to merge 162 commits into from
Closed

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Oct 21, 2020

#26751 Add duotone to image block
#26752 Add duotone block supports

⚠️ Do not merge this PR—the ones linked above should be be the ones merged. ⚠️

Since this is a rather large PR with a long history, I've simplified it down into two smaller PRs, #26751 and #26752, that can be run and reviewed independently of one another.

Both PRs share a few commits for the duotone component, toolbar, and theme.json code. I'll continue to add code to this PR which has all the history and apply those changes to the individual commits on the respective smaller PR.

#26751, the image block PR, is a bit more straightforward, so I recommend reviewing that first. #26752 requires some additional changes to core in WordPress/wordpress-develop#984, and it involves more blocks.

Description

Adds duotone filters as a block-supports feature—originally prototyped on Automattic/block-experiments. With the SVG approach, there is wide browser support, so it'll also be supported on IE11.

The duotone feature comes with its own default palette. And a new option for duotone has been added to theme.json. See theme-json.md for details.

Adding it to a block is done with the supports.duotone option in the block.json. See block-supports.md for details.

In the future this can be expanded to more than two colors. That's mostly a UI problem right now, the feature supports arrays with more than two colors on the backend.

Support for the image block is added in this PR. Adding it to the cover block is blocked by #25171. Additionally the video block, cover block, and media & text block have support added via block supports which depends on WordPress/wordpress-develop#984.

How has this been tested?

  1. Create an image block, video block, cover block, and media & text block
  2. Apply duotone filter from presets, the bar, and the palette selectors
  3. Save and preview the duotone blocks
  4. Convert between the supported blocks to see that duotone is maintained across the transformation
  5. Test that you can add duotone with keyboard only

Screenshots

Old screenshots

duotone toolbar and picker duotone in the cover block duotone in the image block duotone in the media & text block duotone in the video block

media-text-duotone-2020-11-05

image-duotone-2020-11-05

duotone

@ajlende ajlende self-assigned this Oct 21, 2020
@ajlende ajlende added [Block] Image Affects the Image Block [Feature] Media Anything that impacts the experience of managing media [Type] New API New API to be used by plugin developers or package users. labels Oct 21, 2020
@ZebulanStanphill
Copy link
Member

If this supports.duotone feature could support 3 or more tones in the future, shouldn't it be called supports.multitone or something similar instead?

@ajlende
Copy link
Contributor Author

ajlende commented Oct 21, 2020

@ZebulanStanphill I thought about that—technically "gradient map" is the most accurate term but I decided to leave "duotone" because it's a more popular search term. I figured leaving it as "duotone" would improve discoverability even though it can do more than just two colors. I'm still open to changing it, though. Thanks for bringing that up!

@jasmussen
Copy link
Contributor

I like "Duotone" for exactly the reason Alex mentions, it feels descriptive even if in some situations it isn't accurate. It's like the Bold button/icon that technically applies the strong tag, not the b tag.

Besides, I suspect the majority of the presets we end up providing in this package are going to be duotones, as it very quickly gets muddy if you add additional stops. It also simplifies the UI rather drastically, which is important for the first version. It's cool that it's possible, though, and that the feature can grow if need be!

@jasmussen
Copy link
Contributor

I think with this PR we can potentially revisit some of the structure of the Image block toolbar, which landed a little complexity with the image tools work.

For starters, I'd love to see us grouping together block level tools in a single toolbar group. This frees up space, reduces borders, and cleans up the room:

Screenshot 2020-10-22 at 12 50 36

In the above, I'm picturing the Duotone filter button living inside the "Edit" menu. On one hand that feels appropriate as the filter is contextually related to the edit tools. But at the same time it buries them one level deep. If we want to go with them directly available (could always put them there for starters), the button should live before the "Replace" button:

Screenshot 2020-10-22 at 12 50 53

Since the duotone is technically a gradient, we could take inspiration from how the gradient tools work to configure gradients:

Screenshot 2020-10-22 at 12 53 56

I don't love how that would open another color popover within the popover, when you need to customize the colors:

Screenshot 2020-10-22 at 12 54 56

so I hope to figure out a way to show the color picker inline (which has its own sets of problems).

But the popover route may be a good way to start, as I would expect us to provide a range of excellent presets, making that the primary interaction.

@ajlende
Copy link
Contributor Author

ajlende commented Oct 22, 2020

For starters, I'd love to see us grouping together block level tools in a single toolbar group. This frees up space, reduces borders, and cleans up the room

Seems out of scope for this PR, since it would probably require refactoring the align, link, and image tools toolbars too. But I agree that would look nicer.

In the above, I'm picturing the Duotone filter button living inside the "Edit" menu...[otherwise] the button should live before the "Replace" button

Looking at how the alignment toolbar is done in the image block, I can export the components separately and allow the block to use them manually with some refactoring.

I think having it outside of the edit/crop menu will be better because it may be confusing that saving the crop with a filter doesn't upload the filtered image and because it's nice to change the filters without the crop overlay.

Currently, the toolbar appears in both places. If I'm refactoring it to be manually added by the image block I can hide it in the edit/crop menu.

duotone in cropper

duotone after block

As a block filter, there isn't a whole lot of control over placement—it gets added to the toolbar either before or after all the existing toolbar items. In that case (for blocks that are don't manually add the duotone toolbar), do you think before or after would be better?

duotone after block

duotone before block

Since the duotone is technically a gradient, we could take inspiration from how the gradient tools work to configure gradients

In an earlier iteration, I tried reusing the gradient picker and parsing out the colors. I converted calculations into sRGB space instead of linearRGB space to match how CSS gradients are calculated (even if it's not the "correct" way to blend colors). This means that we could show the full gradient transition rather than the hard break if you wanted and it would still be color-accurate.

The SVG feComponentTransfer doesn't have explicitly defined stops for the colors—instead equidistantly spacing them—so we'd probably want to lock the stops in place. Between that and storing the duotone data differently, we're not going to get much reuse out of the existing gradient picker as far as code, unfortunately.

I don't love how that would open another color popover within the popover...so I hope to figure out a way to show the color picker inline

The inline color picker would be nice in all places the popover is currently used, so a design for that could supersede #26115. For now I'll continue working on #26115 since that'll fix the major accessibility concerns I have with doing the popover. I don't want this to ship before the accessibility issues have been resolved.

@jasmussen
Copy link
Contributor

Great thoughts. I'll look into a separate PR or issue about potentially grouping the block level stuff together.

Ugh the edit tools menu definitely is a bit of a mess. I don't actually know what I like better — the duotone button after "Replace", or inside the "Edit" menu. Are you sure there's no way we could put it before "Replace" in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Feature] Media Anything that impacts the experience of managing media [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants