Design Tools: Broken layout in color panel

[aaronrobertshaw]

Description

If no text color has been selected, but a background color has been, when first switching to the styles tab, there'll be a gap between the text and background buttons in the color panel. As soon as something in the inspector is interacted with the gap will disappear.

A git bisect points to the style changes introduced in #55207.

cc/ @andrewserong and @ciampo

Step-by-step reproduction instructions

1. Edit a post, add a group block, select it and switch to the styles tab
2. Add a background color
3. Deselect the block and reselect it
4. Switch to the styles tab and note the gap between text and background controls
5. Click somewhere in the inspector and note the gap disappears

Note: Enabling the "Emulate a focused page" rendering option in Chrome devtools helps debug and inspect styles for this issue.

Screenshots, screen recording, code snippet

Screen.Recording.2023-11-23.at.6.56.26.pm.mp4

Environment info

• WordPress 6.4
• Gutenberg trunk
• TT3

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[aaronrobertshaw]

Removing this top margin on the first color gradient settings item appears to remove the gap ok but there's still a double up of the border. Until the browser redraws this panel.

We might also be able to remove the top margin reset in the color hooks styles.

Also, on closer inspection, it looks like the background item has its top border radii set incorrectly too until the redraw.

[ciampo]

This looks very glitchy 🤔

• the bug seems to happen only sometimes, and seems to appear/go away based when the UI re-renders
• those styles are based on the nth-child selector, so I wonder if we should watch for changes in the DOM structure to spot if there's anything that we didn't consider when writing those CSS selectors

Does this happen in all browsers?

[andrewserong]

Thanks for writing this issue up, and for coming up with reproducible steps! I noticed it while testing out appearanceTools in Classic themes (#56131) but couldn't reproduce it in a blocks-based theme, so assumed it was something particular to that feature. I can reproduce it in any theme based on your instructions now, though — it happens consistently for me with a Group block with Background color and no Text color, and it seems only on blocks with tabbed settings/styles (for me at least).

Does this happen in all browsers?

I can only reproduce it on Google Chrome and Edge on Mac. Firefox and Safari appear to be okay as far as I can tell.

Note: Enabling the "Emulate a focused page" rendering option in Chrome devtools helps debug and inspect styles for this issue.

TIL, thank you! That's super helpful.

[andrewserong]

Unfortunately I haven't had much luck trying to come up with a solution. It's so weird how the Background panel item seems to think (in Chrome at least) that it's still the &:nth-child(1 of &) { until we interact with that area to force a repaint of some kind 🤔

For a moment I was wondering if we could implement another first and last class solution (particular to the color gradient settings items) over in the ColorPanel based on the position in the length of the array of items, but that won't work due to potentially injected color panel items (i.e. overlay color, etc) so we don't have anywhere with a canonical array of all the color-based toolspanel items for a JS alternative to the CSS rule... 😞

This has me stumped for the moment, I'm sorry!

[ciampo]

Took a quick look, and this definitely looks like a browser glitch.

• I am debugging this by enabling the "Emulate a focused page" option in Chrome dev tools
• When inspecting the tools panel item, I can't even see the CSS rule applying the margin, even though the item has a computed margin top
• Literally any action that causes the browser to re-render the component (loss of window focus, window resize, CSS changes...) removes the glitch

Kapture.2023-11-24.at.13.34.07.mp4

The only "hint" that we could look further into is why this happens only when a color is set — what is the difference there?

My gut tells me that this could be related to some race condition between react updates and re-renders. We may have to change some useEffects to useLayoutEffects, or even test some flushSync calls.

[t-hamano]

Does this happen in all browsers?

I also tested this issue on Windows OS and got the following results:

Reproduced

• Chrome 119.0.6045.160 (Official Build) (64-bit)
• Chrome 121.0.6145.0（Official Build）canary (64bit)
• Edge: 119.0.2151.72 (Official build) (64-bit)

Not Reproduced

• Firefox: 120.0 (64-bit)

[t-hamano]

Another thing I found out is that this also happens with other blocks, and happens when the last item in the color panel has a color.

Cover Block

Social Icons Block

[ciampo]

Ok, so to recap:

• the glitch shows only on the last visible toolbar item
• the glitch shows only when the item has a set color value
• the glitch happens only when re-loading the styles panel and a color is already set (it doesn't happen when setting a color)
• the glitch happens only in chromium-based browsers

My gut tells me that this could be related to some race condition between react updates and re-renders. We may have to change some useEffects to useLayoutEffects, or even test some flushSync calls.

I don't have much time to deep dive into debugging this, but if I did, this is what I'd look into at first.

[t-hamano]

I don't fully understand these components and may cause unintended problems, but the following changes seem to solve the problem.

diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.ts b/packages/components/src/tools-panel/tools-panel-item/hook.ts
index 23701afdfc..816b316802 100644
--- a/packages/components/src/tools-panel/tools-panel-item/hook.ts
+++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts
@@ -2,7 +2,7 @@
  * WordPress dependencies
  */
 import { usePrevious } from '@wordpress/compose';
-import { useCallback, useEffect, useMemo } from '@wordpress/element';
+import { useCallback, useEffect, useLayoutEffect, useMemo } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -59,7 +59,7 @@ export function useToolsPanelItem(
 
        // Registering the panel item allows the panel to include it in its
        // automatically generated menu and determine its initial checked status.
-       useEffect( () => {
+       useLayoutEffect( () => {
                if ( hasMatchingPanel && previousPanelId !== null ) {
                        registerPanelItem( {
                                hasValue: hasValueCallback,

[ciampo]

I haven't tested your diff, but this definitely aligns with my gut feeling that this glitch is caused by some race condition between component logic and when the component gets rendered to the DOM (this article helps to highlight the differences between the two).

I don't have capacity before EOW — in terms of coordination, let me know if anyone can work on a PR with the fix, otherwise I'm happy to do it next week.

[aaronrobertshaw]

Thanks for the debugging everyone and the suggested fix @t-hamano.

I've applied Aki's patch and given it a test locally. It appears to be working well for me so I spun up #56536.

I also took a quick look at the primary ToolsPanel hook to see what if anything there should also be switched to useLayoutEffect but nothing jumped out or appear to make a difference.