Avoid paint on popover when hovering content

[corentin-gautier]

What?

Popovers use outline instead of border but it seems to trigger paint when hovering elements inside the popover

Why?

Using border avoids triggering paint which is better for performances

How?

Replace the use of outline by border

Testing Instructions

1. Open the developer console and enable paint flashing
2. Open a color palette popover
3. Hover over a circular picker
4. There should not be paint of the entire popover area

Screenshots

Before

Screen.Recording.2022-11-30.at.14.01.11.mov

After

Screen.Recording.2022-11-30.at.14.02.35.mov

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @corentin-gautier! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[ciampo]

@jasmussen do you have any context to why we may want to use outline instead of border for Popover? This PR is proposing a rendering perf improvement, but it uses border instead of outline.

[jasmussen]

Based on this comment here, https://github.com/WordPress/gutenberg/pull/40740/files#diff-7895f8092f907d624abdbe647e2201f1cebe9198cd3fae6ba759b70a4c0bfef4R16, it appears to be related to popover positioning computations. @youknowriad might have more context, but it sounds like the math that positions the inserter benefits from the outline not counting towards the block footprint.

An alternate solution, not sure if it affects perf, is to use an inner box-shadow for the border. That should theoretically accomplish the same.

[ciampo]

As @jasmussen says, we can't use border because it affects the computed position for the Popover

This PR	trunk

I tried using box-shadow on my machine as suggested:

Click to see the code changes I applied

diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss
index f91f08b84f..c99034234d 100644
--- a/packages/components/src/popover/style.scss
+++ b/packages/components/src/popover/style.scss
@@ -1,5 +1,10 @@
 $arrow-triangle-base-size: 14px;
 
+$shadow-popover-border-default: 0 0 0 $border-width $gray-400;
+$shadow-popover-border-default-alternate: 0 0 0 $border-width $gray-900;
+$shadow-popover-border-top-only: 0 #{-$border-width} 0 0 $gray-400;
+$shadow-popover-border-top-only-alternate: 0 #{-$border-width} 0 $gray-900;
+
 .components-popover {
 	z-index: z-index(".components-popover");
 
@@ -15,16 +20,16 @@ $arrow-triangle-base-size: 14px;
 
 .components-popover__content {
 	background: $white;
-	border: $border-width solid $gray-400;
-	box-shadow: $shadow-popover;
+	// Using `box-shadow` instead of `border` to avoid impacting
+	// popover computations.
+	box-shadow: $shadow-popover-border-default, $shadow-popover;
 	border-radius: $radius-block-ui;
 	box-sizing: border-box;
 	width: min-content;
 
 	// Alternate treatment for popovers that put them at elevation zero with high contrast.
 	.is-alternate & {
-		border-color: $gray-900;
-		box-shadow: none;
+		box-shadow: $shadow-popover-border-default-alternate;
 	}
 
 	// A style that gives the popover no visible ui.
@@ -41,7 +46,11 @@ $arrow-triangle-base-size: 14px;
 		overflow-y: visible;
 		width: auto;
 		border: none;
-		border-top-color: $gray-900;
+		box-shadow: $shadow-popover-border-top-only, $shadow-popover;
+	}
+
+	.components-popover.is-expanded.is-alternate & {
+		box-shadow: $shadow-popover-border-top-only-alternate, $shadow-popover;
 	}
 }

From a quick look:

• paint flashes are still reduced as per this PR's intention
• Popover looks like on trunk (and the new border still doesn't affect the Popover's position)

@corentin-gautier , do you mind applying the suggested changes from the code snippet above and check if it achieves the rendering perf improvement that you had in mind?

[corentin-gautier]

@ciampo @jasmussen box-shadow doesn't trigger paint 👍

[ciampo]

Before merging, do you mind adding a CHANGELOG entry for the changes in the @wordpress/components package, as suggested by Aaron in this other PR?

[ciampo]

@mirka or @tyxla do you mind giving this PR a final round of review? I'm not going to be around in the upcoming weeks

[tyxla]

This looks pretty good! Thanks @corentin-gautier 🙌

Let's have a CHANGELOG entry and I'm happy to ship it afterwards.

[corentin-gautier]

This looks pretty good! Thanks @corentin-gautier 🙌

Let's have a CHANGELOG entry and I'm happy to ship it afterwards.

Done !

[tyxla]

This looks pretty good! Thanks @corentin-gautier 🙌
Let's have a CHANGELOG entry and I'm happy to ship it afterwards.

Done !

Thanks, @corentin-gautier, but could you please rebase the branch to the latest trunk as well? It seems to have some conflicts currently.

[corentin-gautier]

@tyxla and done ;)

[tyxla]

Thanks again @corentin-gautier 🙌 Let me ship this for you.

[tyxla]

✅