Sites Management Page: Use <DropdownMenu /> core component

[danielbachhuber]

Fixes #66725

Proposed Changes

Uses the <DropdownMenu /> core component instead of Calypso's <EllipsisMenu />.

The icon looks a little different than the original issue because P2 uses its own ellipsis icon and I opted to save bundle size by reusing an existing <Gridicon />.

After

Before

Testing Instructions

1. Navigate to /sites-dashboard.
2. Verify the dropdown menu appears and works as expected.

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-40755

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-40755&env=jetpack

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~1190 bytes added 📈 [gzipped])

name             parsed_size           gzip_size
sites-dashboard      +2806 B  (+1.4%)    +1190 B  (+1.8%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[vykes-mac]

Seems like the favoured component to use. One thing I don't like though is that the highlighter remained on the first menu item even when I hover other items, I think the highlighter should move to the item I hover.

Screen.Recording.2022-08-23.at.11.56.32.AM.mov

[danielbachhuber]

One thing I don't like though is that the highlighter remained on the first menu item even when I hover other items, I think the highlighter should move to the item I hover.

@vykes-mac Looks like there's an existing issue on the topic! WordPress/gutenberg#26401

It seems like there should be a blue hover state. Here's the Block Editor:

CleanShot.2022-08-23.at.10.04.36.mp4

It seems like we're missing the --color-neutral-70 CSS variable for some reason...

[vykes-mac]

I see from the existing issue seems like the blue highlighter is for focus purposes only and the blue text is for hover. It would seem the --color-neutral-70 is what's preventing the blue text when we hover. remove it and the blue text shows up.

[zaguiini]

The table is wider now that we're using a different component.

Adding

inset-inline-end: 18px;
position: relative;

to the dropdown (.components-dropdown components-dropdown-menu) element on the table row fixes it.

Also, adding display: flex to the grid item version, along with

padding-block: 0;
height: auto;

on the button (.components-button.has-icon) restores the same line height for the title element for every grid item. It occupies more vertical space now due to the size of the button.

[danielbachhuber]

The table is wider now that we're using a different component.

@zaguiini Oh, good eye. I missed that before.

I've added a couple of tweaks with 989e791 and f116d3a

Here's how it looks now:

I see from the existing issue seems like the blue highlighter is for focus purposes only and the blue text is for hover. It would seem the --color-neutral-70 is what's preventing the blue text when we hover. remove it and the blue text shows up.

@p-jackson Any good ideas on how to sort out this color mess?

[p-jackson]

Any good ideas on how to sort out this color mess?

Yeah it is a bit of a mess 😮‍💨

Core button styles are defined here: https://github.com/WordPress/gutenberg/blob/8b9db3dc025f73e9dad9b076c6f14394b898a9b5/packages/components/src/button/style.scss#L19-L24
They're using the --wp-admin-theme-color variable for the hover colour.

Over in Calypso we override the core styles in _wp-components-overrides.scss:

wp-calypso/client/assets/stylesheets/_wp-components-overrides.scss

Lines 15 to 18 in 1455eef

	&,
	&:hover:not( :disabled ) {
	color: var( --color-neutral-70 );
	}

That's where --color-neutral-70 comes from. It resets all button colours back to "neutral" before applying our Calypso variables on top. Presumably so it matches the user's chosen UI theme.

If it's a "primary" button it applies one set of styles. If it's a "secondary" button it applies a different set of styles. Same thing for "tertiary".

But then that's it, it doesn't apply any other styles, so any other button variant doesn't get styled.

I thought I'd just have to add a new override for whatever variant type the menu item is, but alas <MenuItem> has no variant (it's not primary, tertiary, link, or anything). So I might have to do something hackier.

Does @Automattic/team-calypso-components have any advice?

[p-jackson]

I've just pushed an override that fixes hover state just for our menu items.

But now that I think of it, maybe we should add the styling for .components-menu-item__button to _wp-components-overrides.scss 🤔

[ciampo]

I'm not really familiar with all the style overrides in Calypso, but are all of them necessary at all? Can't Button be used as-is?

Ideally, we should avoid overriding component styles as much as possible. Also, those classnames are not guaranteed to be public APIs, and relying on them is definitely prone to style breakage over time.

We're soon going to the experiment with adding some (simple) theming options to components, which hopefully will remove the need for the majority of the style overrides

[p-jackson]

which hopefully will remove the need for the majority of the style overrides

That sounds great! My guess is the overrides are needed because the core components don't use enough variables.

For example, rather than overriding the color: var(--wp-admin-theme-color); rule that <Button> uses on hover, we could instead redefine the --wp-admin-theme-color variable to match the user's theme preference. However, the non-hover colour doesn't use a CSS variable, and so we can't tweak it the same way. But maybe that just needs a PR to core.

[ciampo]

But maybe that just needs a PR to core.

Definitely! Feel free to open a PR (or even just an issue) in core and tag @mirka and myself :)

[zaguiini]

A few changes but overall good to go!

[a8ci18n]

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7493951

Thank you @danielbachhuber for including a screenshot in the description! This is really helpful for our translators.

[a8ci18n]

Translation for this Pull Request has now been finished.