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: Duotone filter button should always be at the end #31373

Open
Tracked by #33447
jasmussen opened this issue Apr 30, 2021 · 23 comments
Open
Tracked by #33447

Duotone: Duotone filter button should always be at the end #31373

jasmussen opened this issue Apr 30, 2021 · 23 comments
Labels
[Block] Image Affects the Image Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended

Comments

@jasmussen
Copy link
Contributor

jasmussen commented Apr 30, 2021

The Duotone button should always be at the end. Right now it starts as the first button, then if you deselect the block and select it again, it moves to the end:

end

This ticket has been updated to focus on just the one remaining item.

@jasmussen jasmussen added [Feature] Media Anything that impacts the experience of managing media [Block] Image Affects the Image Block CSS Styling Related to editor and front end styles, CSS-specific issues. labels Apr 30, 2021
@jasmussen
Copy link
Contributor Author

@ajlende want to pair with me on this one?

@ajlende
Copy link
Contributor

ajlende commented May 1, 2021

I'll get a branch going for the setup state first. There's a chance that fixing that will fix the reordering issue since the first render will happen when the rest of the toolbar buttons are added. 🤞

@mtias mtias added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label May 14, 2021
@jasmussen
Copy link
Contributor Author

In case it's related to the order in which it shows up and in the placeholder, I'm noticing a jumpy toolbar when applying the filter:

currently

Alex do you know what this might be caused by?

@walbo
Copy link
Member

walbo commented Jun 18, 2021

This enhancement should also be added to the cover block. Doutone should only be visible when you have added a background image. Right now it shows when background color is selected istead of an image and the duotone controls doesn't do anything.

@jasmussen
Copy link
Contributor Author

jasmussen commented Sep 8, 2021

@ajlende I updated this ticket to be checklist format. I also have some initial mockups for a longer term filter menu redesign that both revisits the shadow/highlight buttons, and adds the non-destructive text. The mockups need a bit more thought, but consider it a snapshot of work in progress:

Image Filters i15

@jasmussen jasmussen added the [Type] Overview Comprehensive, high level view of an area of focus often with multiple tracking issues label Sep 8, 2021
@jasmussen
Copy link
Contributor Author

Created a new separate issue for adding a duotone shortcut to the Gallery block: #34950

@jasmussen
Copy link
Contributor Author

I've edited the title and description of this issue:

  • One issue — a description to note the filter is non destructive is already fixed
  • The other issue, that duotone should not be available in the setup state, was recently made intentional for the Featured Image block, so it's clear that a filter will be applied. There are opportunities to expand that for the Image block itself, but I removed it from this one for now to focus this ticket.

The remaining issue: always showing the filter button at the end, is now the focus of this ticket.

@jasmussen jasmussen removed [Type] Overview Comprehensive, high level view of an area of focus often with multiple tracking issues CSS Styling Related to editor and front end styles, CSS-specific issues. labels Apr 27, 2022
@richtabor
Copy link
Member

I'm noticing this more and more — @ajlende is this a big lift?

@richtabor
Copy link
Member

The other issue, that duotone should not be available in the setup state, was recently made intentional for the Featured Image block, so it's clear that a filter will be applied.

I made a separate issue (reference the Cover block) for this here: #48804

@ajlende
Copy link
Contributor

ajlende commented Mar 6, 2023

I'm noticing this more and more — @ajlende is this a big lift?

It isn't so much a duotone problem as a problem with the slot/fill system that can't guarantee rendering order. My suspicion is that, for the first render, the duotone code is being run first which places it at the beginning of the slot, and then on subsequent renders it falls later for some reason.

Removing duotone from the setup state for the cover block was a potential way to sidestep the issue by never calling that first render because fixing the root of the issue seemed much more difficult.

@richtabor
Copy link
Member

Removing duotone from the setup state for the cover block was a potential way to sidestep the issue by never calling that first render because fixing the root of the issue seemed much more difficult.

Gotcha. I don't think that would clear it though — on trunk at least it's consistently first.

CleanShot 2023-03-06 at 17 12 27

@Mamaduka
Copy link
Member

This is a general problem with SlotFills and will be fixed once we have a solution for #15641.

@richtabor
Copy link
Member

richtabor commented Apr 12, 2023

This is a general problem with SlotFills and will be fixed once we have a solution for #15641.

Would this result affect block controls as well? How do you suggest we move forward?

If anything, if we can at least keep the "Align" icon control, that would be acceptable. That's the inconsistency with all other blocks that support alignment.

Is there a temporary fix we could put in to do that you think? cc @ajlende

@Mamaduka
Copy link
Member

The general fix will benefit both since block/inspector controls are rendered using SlotFills.

There're some "hacky" workarounds for the bug, but I don't think we can apply it here.

I think the Duotone control position is more consistent on the current trunk than when the issue was initially reported. We can close this duplicate of #15641.

@richtabor
Copy link
Member

I think the Duotone control position is more consistent on the current trunk

Yes, but it's consistently where the Align control should be, which is consistently first in blocks that don't support duotone.

@richtabor
Copy link
Member

I think it's fine to keep this separate/open perhaps, as it's more specific.

@ajlende
Copy link
Contributor

ajlende commented Apr 13, 2023

Is there a temporary fix we could put in to do that you think?

@Mamaduka is right, the hacks he's mentioning are for fixing asynchronous rendering by creating a placeholder element in the DOM that can get swapped when the async part resolves.

The only hack I can think of is to purposely delay duotone rendering by a fixed amount so it pops in half a second after opening the toolbar or something, but that sounds worse than what we have to me. Furthermore, if you try to do that for a second toolbar item, the problem will pop up again.

@Mamaduka
Copy link
Member

I think it's fine to keep this separate/open perhaps, as it's more specific.

Sure. But the details here aren't related to the actual problem, just symptoms of it.

@Mamaduka
Copy link
Member

Mamaduka commented Jun 1, 2023

I can no longer reproduce the issue on Gutenberg trunk; the Duotone filter is always the first item in the toolbar.

It's still an issue on WP 6.2.2.

@richtabor
Copy link
Member

the Duotone filter is always the first item in the toolbar.

It needs to be the last one though, or at least after alignment.

CleanShot 2023-06-01 at 08 59 15

@Mamaduka
Copy link
Member

Mamaduka commented Jun 1, 2023

I mean first from the left 😅

@richtabor
Copy link
Member

Yea, it's consistently the first from the left, but it should be anywhere else ha — at the end would be best I think.

@Mamaduka
Copy link
Member

Mamaduka commented Jun 1, 2023

The issue is back if I move it at the end by rendering the block edit component first and then Duotone controls.

@jordesign jordesign added the [Type] Bug An existing feature does not function as intended label Sep 8, 2023
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] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended
Projects
Status: Not Started
Development

No branches or pull requests

7 participants