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

Aspect ratio support breaks child theme CSS when unnecessary #60922

Open
eric-michel opened this issue Apr 19, 2024 · 3 comments
Open

Aspect ratio support breaks child theme CSS when unnecessary #60922

eric-michel opened this issue Apr 19, 2024 · 3 comments
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Bug An existing feature does not function as intended

Comments

@eric-michel
Copy link

Description

WP 6.5 introduced the a new aspect-ratio tool to the Cover block. It breaks existing child theme CSS that sets aspect-ratio on the Cover block even when the control is unused in the editor.

When a custom min-height is selected in the Cover block controls, an inline style of aspect-ratio: unset; is output into the Cover block. This will override any custom CSS applied to the block that controls aspect-ratio within the theme.

This breaks all of our block theme sites. We apply a custom min-height in the block controls and an aspect-ratio property via a custom class to most of our Cover blocks.

Problems with CSS specificity been discussed ad nauseam, so I won't go on for too long, but even if we accept that of course the editor will have CSS changes from time-to-time, adding an inline unset of a property that isn't being set in the editor is incredibly frustrating.

Long term, I'm confident that eventually cascade layers will solve this issue, but as @cbirdsong has rightly pointed out, there is no polyfill or progressive enhancement we can use to incrementally implement them, so we have to wait for widespread browser adoption. In the meantime, there has to be more attention given to how CSS changes are applied.

Step-by-step reproduction instructions

  1. Install WP 6.5
  2. Add a cover block with a custom min-height set in the block controls
  3. Inspect the frontend of the block and notice that the main Cover block container now unset aspect-ratio

Screenshots, screen recording, code snippet

No response

Environment info

WP version 6.5.2, any block theme.

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

@eric-michel eric-michel added the [Type] Bug An existing feature does not function as intended label Apr 19, 2024
@cbirdsong
Copy link

cbirdsong commented Apr 20, 2024

We ran into a similar issue when aspect ratio support was added to the image block, which also introduced inline CSS. A theme should easily be able to opt out of all new editor features on existing blocks. I do not have the capacity to go back and add whatever: false to the theme.json file on every theme I’ve worked on every time there’s a major new WP version.

Also, related: Inline CSS continues to be a terrible thing for a CMS that powers 40% of the Internet to build its editor on. There are so many reasons it should be avoided:

Relatedly, on this point:

Long term, I'm confident that eventually cascade layers will solve this issue.

You can’t use cascade layers with inline CSS. I order to adopt them, every bit of inline CSS will need to be reworked to use classes in some capacity, but usage of inline CSS is only increasing.

@jordesign jordesign added the [Block] Cover Affects the Cover Block - used to display content laid over a background image label Apr 29, 2024
@eric-michel
Copy link
Author

You can’t use cascade layers with inline CSS. I order to adopt them, every bit of inline CSS will need to be reworked to use classes in some capacity, but usage of inline CSS is only increasing.

This is a great point that I kind of glossed over. Inline styles have definitely caused a considerable number of issues. In this case, I think it's particularly egregious, because it's unsetting a property that isn't being set in the editor. It's one thing to set an inline style property that the user is explicitly setting in the editor where there's a reasonable assumption of that property needing high priority, it's another to unset a property that isn't be interacted with at all.

It's getting to the point that I feel like I should just stack all our sites' CSS files with !important flags just in anticipation of this happening every release cycle.

@eric-michel
Copy link
Author

@aaronrobertshaw RE: #63345 (comment)

Leaving aside the aspect ratio issue for a moment, regarding inline styles, I'd see part of the effort of implementing CSS layers in WordPress as working out if inline styles should/could also be migrated into a separate stylesheet and CSS layer. It's unclear at the moment how much of a difference that might make as these are styles specific to a block instance and "should" override all other theme and global styles as they are applied by the end user.

I am somewhat ambivalent about the use of inline styles generally. If I set the top padding of a block to medium and it adds an inline style to that block that sets padding-top to medium, that seems ok to me because I am making a very specific, unambiguous choice about what I want that padding to be. So I want to be clear that my complaint in the other thread was not about the existence of inline styles overall (although I know others make good arguments against them).

My problem in this issue is: an inline style is being added to set the aspect-ratio property even when no choice is made to select an Aspect Ratio in the related setting for the block. It is doing so based on an unrelated setting (Minimum Height). This is similar to my complaint in #64276. Having the editor apply CSS properties to blocks unnecessarily provides zero benefit and only introduces potential conflicts with themes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants