Twenty Twenty-One: Dropdown arrow missing in the widget editor

[zieladam]

In the widget editor, the <select /> arrow disappears when focused when twentytwentyone theme is active:

​https://user-images.githubusercontent.com/205419/123951705-5f803100-d9a5-11eb-92c6-94d072e01d8a.png

This is caused by reset.css overriding style-editor.css from the theme:

/* reset.css */
.editor-styles-wrapper select:focus {
    color: revert;
    border-color: revert;
    background: revert;
    box-shadow: revert;
    text-shadow: revert;
    cursor: revert;
    transform: revert;
}

/* style-editor.css */
.editor-styles-wrapper select {
        border: var(--form--border-width) solid var(--form--border-color);
        border-radius: var(--form--border-radius);
        color: var(--form--color-text);
        font-size: var(--form--font-size);
        -moz-appearance: none;
        -webkit-appearance: none;
        appearance: none;
        padding: var(--form--spacing-unit) calc(3 * var(--form--spacing-unit)) var(--form--spacing-unit) var(--form--spacing-unit);
        background: var(--global--color-white) url(data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>) no-repeat;
        background-position: right var(--form--spacing-unit) top 60%;
}

The rogue reset.css rule was introduced in ​https://github.com/WordPress/gutenberg/pull/30141 to remove the opinionated style from the theme. At the same time, we want to use at least some theme styles here. There are two ways to approach this:

1. Fully rely on wp-admin core styles when it comes to form elements in the widgets editor with "use theme styles" feature enabled
2. Make the theme CSS more specific to overcome the .editor-styles-wrapper select:focus rule from reset.css in this specific case

I went for 2 because it seems safer, especially at RC1/RC2 stage. I don't think this is an ideal solution though – I wonder what y'all think about 1. Would it make sense? Or would it completely defeat the purpose of use theme styles ? Personally I don't mind seeing the "native" wp-admin CSS in the edit mode widgets since, as a user, I mostly care about the preview.

This is how it looks like with ​https://github.com/WordPress/wordpress-develop/pull/1454 applied:

​https://user-images.githubusercontent.com/205419/123951704-5f803100-d9a5-11eb-9366-f22e4bfeba3d.png

Props to @spacedmonkey for reporting this in ​https://github.com/WordPress/gutenberg/pull/33040

Problem:

In the widget editor, the <select /> arrow disappears when focused when twentytwentyone theme is active:

<img width="764" alt="Zrzut ekranu 2021-06-30 o 13 01 10" src="​https://user-images.githubusercontent.com/205419/123951705-5f803100-d9a5-11eb-92c6-94d072e01d8a.png"/>

This is caused by reset.css overriding style-editor.css from the theme.

reset.css

.editor-styles-wrapper select:focus {
    color: revert;
    border-color: revert;
    background: revert;
    box-shadow: revert;
    text-shadow: revert;
    cursor: revert;
    transform: revert;
}

style-editor.css:

.editor-styles-wrapper select {
	border: var(--form--border-width) solid var(--form--border-color);
	border-radius: var(--form--border-radius);
	color: var(--form--color-text);
	font-size: var(--form--font-size);
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: var(--form--spacing-unit) calc(3 * var(--form--spacing-unit)) var(--form--spacing-unit) var(--form--spacing-unit);
	background: var(--global--color-white) url(data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>) no-repeat;
	background-position: right var(--form--spacing-unit) top 60%;
}

The rogue reset.css rule was introduced in ​https://github.com/WordPress/gutenberg/pull/30141 to remove the opinionated style from the theme. At the same time, we want to use at least some theme styles here. There are two ways to approach this:

1. Fully rely on wp-admin core styles when it comes to form elements in the widgets editor with "use theme styles" feature enabled
2. Make the theme CSS more specific to overcome the .editor-styles-wrapper select:focus rule from reset.css in this specific case

I went for 2 because it seems safer, especially at RC1/RC2 stage. I don't think this is an ideal solution though – I wonder what y'all think about 1. Would it make sense? Or would it completely defeat the purpose of use theme styles ? Personally I don't mind seeing the "native" wp-admin CSS in the edit mode widgets as I mostly care about the preview.

Test plan:

1. Activate TwentyTwentyOne theme
2. Install Web stories plugin
3. Go to the widgets editor
4. Add "Web stories" widget
5. Put focus on any select in the form
6. Confirm the dropdown arrow is visible

<img width="726" alt="Zrzut ekranu 2021-06-30 o 13 00 03" src="​https://user-images.githubusercontent.com/205419/123951704-5f803100-d9a5-11eb-9366-f22e4bfeba3d.png">

Trac ticket: #53560

[desrosj]

I think these changes were reverted accidentally when updating the ticket description.

[desrosj]

@poena Do you have any thoughts here?

[poena]

I don't have time to look into the issue or the PR within the 5.8 time frame.

-The reset.css is not likely to be reverted.

-If this is a third party plugin issue, not reproducable without the plugin, then it should be fixed in the plugin.
So, are core (widget) select lists still working?

-Accessibility is more important than matching the styles for the editor and front.

[isabel_brison]

-If this is a third party plugin issue, not reproducable without the plugin, then it should be fixed in the plugin.
So, are core (widget) select lists still working?

It's not only third party plugins. The issue can be reproduced on the Navigation legacy widget: if there is more than one menu, a select will appear.

-Accessibility is more important than matching the styles for the editor and front.

The issue isn't matching styles in editor and front end, it's that all legacy widget selects in the editor inherit some of the theme styles, but in this case the select background image is being overridden by the editor reset styles on focus. The only thing Adam's PR does is make sure that background image style is also applied to the select in its focused state, which doesn't interfere with theme focus styling in any way, so is unlikely to create an accessibility issue. It's also a lower impact change than removing styles from the editor reset stylesheet would be, which is important at this point in the release cycle.

[poena]

Isabel I never wrote that there was an accessibility problem with the PR,-I specifically wrote that I did not have time to look at the PR.

Accessibility is most important, please interpret that as:
-My thought on the issue, as requested.
-My opinion is that the arrow must always be visible. That anything else is secondary.

[zieladam]

Since @isabel_brison approved the Github PR and the resolution is consistent with @poena guidelines (that the arrow is visible), I'll add the commit keyword here

[desrosj]

The change in the PR will also need to be made to assets/sass/04-elements/forms-editor.scss, otherwise it will be removed when npm run build is run next in the theme. That will ensure the change is applied to the ie-editor.css file as well.

I can handle that before committing!

[desrosj]

In 51296:

Twenty Twenty-One: Ensure the dropdown arrow displays for <select> elements when focused.

Props isabel_brison, zieladam, poena.
Fixes #53560.

[desrosj]

Reopening for backport consideration.

[zieladam]

Thank you @desrosj !

[jorbin]

Looks good for backporting

[SergeyBiryukov]

It seems weird that these rules are fully duplicated in the assets/css/ie-editor.css file:

select {
	border: 3px solid #39414d;
	border-radius: 0;
	color: #28303d;
	font-size: 1.125rem;
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: 10px 30px 10px 10px;
	background: #fff url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right 10px top 60%;
}

select:focus {
	border: 3px solid #39414d;
	border-radius: 0;
	color: #28303d;
	font-size: 1.125rem;
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: 10px 30px 10px 10px;
	background: #fff url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right 10px top 60%;
}

while they are properly combined in assets/css/style-editor.css:

select,
select:focus {
	border: var(--form--border-width) solid var(--form--border-color);
	border-radius: var(--form--border-radius);
	color: var(--form--color-text);
	font-size: var(--form--font-size);
	-moz-appearance: none;
	-webkit-appearance: none;
	appearance: none;
	padding: var(--form--spacing-unit) calc(3 * var(--form--spacing-unit)) var(--form--spacing-unit) var(--form--spacing-unit);
	background: var(--global--color-white) url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='10' height='10' fill='%2328303d'><polygon points='0,0 10,0 5,5'/></svg>") no-repeat;
	background-position: right var(--form--spacing-unit) top 60%;
}

Looking closer, ie-editor.css has a lot more unnecessarily duplicated rules, for example:

.has-background .has-link-color a {
	color: #28303d;
}

.has-background.has-link-color a {
	color: #28303d;
}

I would expect to see this instead:

.has-background .has-link-color a,
.has-background.has-link-color a {
	color: #28303d;
}

Could it be an issue with PostCSS configuration? I've noticed that postcss.config.js for the theme requires the postcss-discard-duplicates module, but it seems like it's not being used, or I'm misunderstanding how it works.

This should probably be investigated in a new ticket though, and should not be a blocker for backporting [51296].

[desrosj]

In 51336:

Twenty Twenty-One: Ensure the dropdown arrow displays for <select> elements when focused.

Props isabel_brison, zieladam, poena.
Merges [51296] to the 5.8 branch.
Fixes #53560.

[desrosj]

I've opened up #53605 to investigate the duplicate rules flagged above.

Merged into Core in https://core.trac.wordpress.org/changeset/51288.