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

SPT: Menu template butting up against viewport sides on smaller screens #39804

Closed
retrofox opened this issue Mar 2, 2020 · 11 comments
Closed

Comments

@retrofox
Copy link
Contributor

retrofox commented Mar 2, 2020

Although the large preview of the Menu template is good (width 1200px), once inserted into the Post Editor canvas the template itself doesn't look right in narrower viewport as the Blocks butt up against the edges of the canvas.

SPT Block Preview
image

Post Editor Canvas
Screen Shot 2020-03-02 at 6 47 19 AM

@getdave getdave self-assigned this Mar 2, 2020
@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

I'll look at this

@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

@retrofox Which Theme are you using?

It was Mayland

@retrofox
Copy link
Contributor Author

retrofox commented Mar 2, 2020

@retrofox Which Theme are you using?

It was Mayland

Yes, but I think it doesn't depend on the theme since guessing that the issue happens at a template definition level.

@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

Analysis: Front end styles

I've started looking into this on the "front end" (user facing, not the Editor).

It appears that this is due to the alignfull setting on the Columns Block that is used to create the Menu layout.

The alignfull class has:

  • a max-width: 100%
  • auto setting for horizontal margins
  • no horizontal padding
  • a complex width calculation

This means that at larger viewport sizes the width setting constrains the Menu layout so that it is "cented" in the viewport. This is good.

However, at smaller viewports, the width setting is larger than the viewport which means the max-width kicks in to stop the content bleeding outside the viewport area. However, as the max-width is 100% it means the edges of the Menu butt up against the edges of the viewport.

To clarify, unlike the alignfull setting which should butt up against the edges at all viewports, the alignwide setting should not butt up against the edges of the viewport on smaller viewports.

Examining the layout further we see that if the alignwide setting is removed then the Columns obey the layout rule established by the top parent site-main <div>. This has 16px of horizontal padding which ensures that no content butts up against the viewport.

If we apply the 16px of padding to the alignwide setting then on small viewports the Menu does not butt up against the edges.

Screenshot 2020-03-02 at 10 50 37

Suggested fix is to see whether we can alter the align-width Sass mixin in Varia to add a 16px margin. If not then we'll have to fix on a Theme level.

@nielslange
Copy link
Member

This issue seems to be related to Automattic/themes#1773

@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

May be fixed by WordPress/gutenberg#20589

@getdave getdave changed the title SPT: Menu template SPT: Menu template butting up against viewport sides on smaller screens Mar 2, 2020
@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

Update

We have a few options to fix this:

  1. Applying a patch directly in SPT. However, as this is not direclty a SPT issue (it happens in the Editor) I think this would be unwise.
  2. Applying a patch to Varia Theme. Not sure of the implications of fixing all Columns, but this is much better than option 1 above.
  3. Fix in Core. This is the root cause of the issue. It depends on whether Core feel this is something that they need to solve, or whether it's for Themes to choose whether they want spacing to the left/right of the Block on small viewports.

I've started on a fix in Core to see if we can fix the root cause and pinged the Themes team for advice on Varia fixes (I understand that @allancole will advise).

@getdave
Copy link
Contributor

getdave commented Mar 3, 2020

I'm not 100%, but @jasmussen has addressed a regression on Core Columns that might be causing this:

WordPress/gutenberg#20589 (review)

Will need testing against our Themes to check this resolves things. This means:

  • Set up local WP install
  • Pull in latest Gutenberg from Github master (it's not released yet!)
  • Pull in latest Automattic Themes
  • Activate Themes and test to see if Menu Layout Template is fixed.

@getdave getdave removed their assignment Mar 9, 2020
@getdave
Copy link
Contributor

getdave commented Mar 9, 2020

@ianstewart This should be fixed by this PR WordPress/gutenberg#20597 as of Gutenberg 7.7 being released to WPCom.

Would you be able to re-test this once Gutenberg 7.7 is available?

cc @iamtakashi

@ianstewart
Copy link
Contributor

@getdave for sure.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been marked as stale and will be closed in seven days. This happened because:

  • It has been inactive in the past 9 months.
  • It isn't a project or a milestone, and hasn’t been labeled `[Pri] Blocker`, `[Pri] High`, `[Status] Keep Open`, or `OSS Citizen`.

You can keep the issue open by adding a comment. If you do, please provide additional context and explain why you’d like it to remain open. You can also close the issue yourself — if you do, please add a brief explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants