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

Radio buttons broken layout within the meta boxes area #18181

Closed
afercia opened this issue Oct 30, 2019 · 6 comments · Fixed by #18183
Closed

Radio buttons broken layout within the meta boxes area #18181

afercia opened this issue Oct 30, 2019 · 6 comments · Fixed by #18183
Assignees
Labels
[Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Oct 30, 2019

Describe the bug
Also reported on Trac as it affects WordPress 5.3 RC 3, see https://core.trac.wordpress.org/ticket/48466

A very recent CSS change in Gutenberg that was merged yesterday in core breaks the radio buttons used by plugins within the editor meta boxes area. Specifically the newly added margin: -3px -4px; makes the radio buttons "blue dot" completely misplaced:

Screenshot 2019-10-30 at 12 15 12

See https://github.com/WordPress/gutenberg/pull/18108/files#diff-0ad88efa3865f313f57c422957d02b2fR99-R101

Removing these negative margins fixes the issue. I'm not sure they are really necessary, as removing them doesn't seem to change anything else. Also, to my understanding #18108 was meant to fix issues within the sidebar, so I'm not sure why it's targeting the meta boxes area.

To reproduce, use Advanced Custom Fields and add a group with a radio button and a checkbox. Or directly edit the HTML in your browser dev tools and add a radio button and a checkbox somewhere within the meta boxes area .postbox.

Screenshot 2019-10-30 at 12 52 21

@afercia afercia added [Priority] High Used to indicate top priority items that need quick attention [Type] Regression Related to a regression in the latest release labels Oct 30, 2019
jasmussen added a commit that referenced this issue Oct 30, 2019
This PR fixes #18181.

The regression was caused due to a margin added to the pseudo element which was not necessary for the radio buttons.
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 30, 2019
@afercia
Copy link
Contributor Author

afercia commented Oct 30, 2019

@youknowriad reminded me plugins and themes can add meta boxes into the default sidebar. See for example the "Astra" theme mentioned in the related issue #18053. Screenshot:

Screenshot 2019-10-30 at 13 13 49

However, the new CSS should not target the meta boxes area below the post.

@jasmussen
Copy link
Contributor

Changed the CSS to not target the meta boxes area below the post.

Screenshot 2019-10-30 at 13 42 32

I noticed that radio buttons have regressed:

Screenshot 2019-10-30 at 13 51 04

I have not slipstreamed a fix in this one as it's an unrelated issue. But the issue is that the radios used to be sized in pixels, now they're sized in rems. In pixels they were 16x16, which made the math to position the dot correct. Now they compute to 15x15px, smaller hit area and obviously non-centered dot.

Is this tracked or is someone working on this?

@afercia
Copy link
Contributor Author

afercia commented Oct 30, 2019

I'm not seeing changes to the the visibility radio buttons. Any special way to reproduce that? Browser?

In pixels they were 16x16 ... Now they compute to 15x15px,

1 rem still computes to 16 pixels. Are you sure the misalignment is not because of some other reasons e.g. a plugin that alters the CSS?

@jasmussen
Copy link
Contributor

Excellent instinct. It's caused by Astra theme, it seems.

@afercia
Copy link
Contributor Author

afercia commented Oct 30, 2019

Oh well.. looks like Astra alters the html base font size... 😬

html {
    font-size: 93.75%;
}

which computes to 15 pixels...

I'd tend to think this deserves a broader discussion. Probably WordPress shouldn't allow themes or plugins to alter the base font size as that would alter any value set in rem by WordPress itself or other plugins and themes.

Per aspera ad Astra.

@jasmussen
Copy link
Contributor

I do agree it is unfortunate that themes can affect the UI as such. The lack of an iframe is causing this. Shadow DOM is an option, rewriting such rules using the built in editor style rewriter is another, but even then themes can enqueue styles globally so we can't fully prevent it.

There is some discussion in #10178 (comment) on how to proceed to protect UI from editor styles.

aduth pushed a commit that referenced this issue Nov 4, 2019
* Fix postmeta radio regression.

This PR fixes #18181.

The regression was caused due to a margin added to the pseudo element which was not necessary for the radio buttons.

* Only apply to side-sortables.
jorgefilipecosta pushed a commit that referenced this issue Nov 5, 2019
* Fix postmeta radio regression.

This PR fixes #18181.

The regression was caused due to a margin added to the pseudo element which was not necessary for the radio buttons.

* Only apply to side-sortables.
jorgefilipecosta pushed a commit that referenced this issue Nov 5, 2019
* Fix postmeta radio regression.

This PR fixes #18181.

The regression was caused due to a margin added to the pseudo element which was not necessary for the radio buttons.

* Only apply to side-sortables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants