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

Editor text has no default left/right margin unless theme opts into site-wide padding #35884

Closed
kjellr opened this issue Oct 22, 2021 · 18 comments · Fixed by #37741
Closed

Editor text has no default left/right margin unless theme opts into site-wide padding #35884

kjellr opened this issue Oct 22, 2021 · 18 comments · Fixed by #37741
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release

Comments

@kjellr
Copy link
Contributor

kjellr commented Oct 22, 2021

If a theme does not opt into site-wide padding through theme.json, and the user does not specify site padding through Global Styles, the post/page editor currently provides no left/right margins to prevent text from bumping up against the edges of the screen:

Screen Shot 2021-10-22 at 11 00 42 AM

This is sort of an issue for block themes, since it effectively requires them to supply either a site-wide padding value, or some custom CSS. More importantly though, it's a regression in the editor experience for users of classic themes that don't manually provide these margins.

To reproduce:

  1. Activate an older theme, like Twenty Eleven.
  2. Open the post editor, and either switch to one of the device previews (tablet/mobile), or make your browser window smaller.
  3. Note that the text bumps against the edges fo the screen.

Gutenberg version: Version 11.8.0-rc.2
Related: WordPress/twentytwentytwo#144

cc @youknowriad since this may be related to removing the site-wide margin.

@kjellr kjellr added the [Type] Regression Related to a regression in the latest release label Oct 22, 2021
@youknowriad
Copy link
Contributor

I think this is a side effect of the removal of the 8px padding we had around the canvas. So how should this work? add this padding for the "post editor" only?

@kjellr
Copy link
Contributor Author

kjellr commented Oct 25, 2021

To start with, we should definitely add it by default for non-block themes. That'll fix the regression aspect.

For block themes, I wonder if it makes sense to add a default padding value back in — when we removed it, there was no site padding control, but now it's a bit easier to specify that. And I think explorations like #35919 will make the control even more useful.

@jasmussen do you have any ideas?

@jasmussen
Copy link
Contributor

Could we add those 8px by default to lib/theme.json? If the "outer padding" aspect gets addressed, then it seems like it might thread the needle between a good default, theme control, and flexibility? Or am I missing a piece?

@kjellr
Copy link
Contributor Author

kjellr commented Oct 25, 2021

Yeah, that's more or less what I'm thinking at this point too.

@kjellr
Copy link
Contributor Author

kjellr commented Nov 3, 2021

@noisysocks I think we should add this to 5.9 must-haves, since it effects a large quantity of themes.

@MaggieCabrera
Copy link
Contributor

I was testing and checking emptytheme and on trunk, with no padding defined, emptytheme doesn't have padding at all on frontend or backend, I think that's the correct behavior. I talked about this with @kjellr and it seems like this is more of a problem for classic themes not using theme.json.

Would the solution be to add this padding in the editor only for classic themes? And let block themes handle this padding or lack thereof via theme.json and ideally #35919 or a similar solution?

@jeffikus
Copy link

jeffikus commented Nov 9, 2021

Would the solution be to add this padding in the editor only for classic themes? And let block themes handle this padding or lack thereof via theme.json and ideally #35919 or a similar solution?

@MaggieCabrera Are you saying add the padding only if theme.json is not present in a classic theme?

@MaggieCabrera
Copy link
Contributor

@MaggieCabrera Are you saying add the padding only if theme.json is not present in a classic theme?

Yeah, and only in the editor, not the frontend, so that it behaves like before the regression, but excluding themes with a theme.json file.

@jeffikus
Copy link

jeffikus commented Nov 9, 2021

@MaggieCabrera that makes sense to me, I agree with that solution.

@kjellr
Copy link
Contributor Author

kjellr commented Nov 9, 2021

I think that makes sense for now as a fix the regression in older themes.

@Mamaduka
Copy link
Member

Mamaduka commented Jan 5, 2022

It looks like we don't have a solution for this issue yet.

I'm going to remove the issue from the 5.9 board. We can try and fix this in the next release.

@kjellr
Copy link
Contributor Author

kjellr commented Jan 5, 2022

I think it's acceptable (but not great) to punt this issue for block themes, but it's not ok to punt the regression that happens for all classic themes. By default today, any theme that provides no editor styles suffers from this issue:

Screen Shot 2022-01-05 at 9 21 46 AM

This effects many of our own recent default themes as well:

Twenty Twenty Twenty Nineteen
2020 2019

This wasn't the case in 5.8, and it's a pretty obvious regression. I think a reasonable fallback would be to push a change that re-imposes the 8px margins from 5.8 for non-block themes.

Is that something we can still fit in?

cc @youknowriad @noisysocks

@scruffian
Copy link
Contributor

Reopening as this was never resolved.

@scruffian scruffian reopened this Mar 3, 2022
@ramonjd
Copy link
Member

ramonjd commented Mar 24, 2022

Adding https://github.com/Automattic/themes/blob/trunk/archeo/style.css as an example of how themes have worked around current issues

props @scruffian

@andrewserong
Copy link
Contributor

andrewserong commented Mar 24, 2022

Just in case it helps for whoever picks up this issue, we currently output styles for the root block selector at this line in the theme JSON class: https://github.com/wordpress/gutenberg/blob/trunk/lib/compat/wordpress-6.0/class-wp-theme-json-gutenberg.php#L106, so that (get_block_classes) could be a good place to put additional styles?

@tellthemachines
Copy link
Contributor

I'm not sure what the intended goal for this issue currently is. The classic theme regression has been fixed, which this comment and following imply is the major concern here.

It would be good to at least be clear about the scope of the problem we're trying to fix: do we want a default padding added for block themes just in the post editor? Or can this be addressed globally as part of #35607?

@kjellr
Copy link
Contributor Author

kjellr commented Mar 28, 2022

I think this could be addressed globally as part of #35607, if that issue implemented a default that wasn't 0.

@scruffian
Copy link
Contributor

I think can close this as it's part of #35607

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release
Projects
None yet
10 participants