Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

Update front end margins to match the editor on desktop screens #502

Merged
merged 9 commits into from
Nov 8, 2018

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Nov 7, 2018

Partially resolves #236

Moves our tablet + desktop margins around a bit so that there's greater alignment between our front and back end. Basically: if you're in full-screen mode in Gutenberg (on a screen above 768px wide), you should see nearly the exact same margins/grid for your post content when you switch to the front end.

  • This is based on the margins/spacing that are built into style-editor.css, so starts by adding 10% left/right margins to .entry-content. There are a few extra pixels added to sync up with Gutenberg's block/container padding. From there, it just modifies the existing margins + max-widths of blocks (and other non-block post elements) so that they fit the new grid.
  • When possible, I switched to using mixins/variables for some of our common body content max-width specs.
  • While I was adjusting the comment margins, I also made it so that our comment reply links don't disappear on small screens. 😉

I ran through my entire test site (including all the theme unit test posts), and things looked fairly solid.

*In order to fully resolve #236, we'll need another PR that syncs up the margins between 600px and 768px viewports.

GIF:

margins

Matches the spacing used in the editor. Only effects breakpoints above 768px.
Eliminate unnecessary styles for blocks, add styles for wp-captions.
So they aren't mistakenly off-centered.
And make sure it doesn't disappear off screen on mobile.
Also, wrap our global styles in an .entry class to make them a little more specific.
@kjellr kjellr added the enhancement New feature or request label Nov 7, 2018
@kjellr kjellr added this to the RC1 milestone Nov 7, 2018
@kjellr kjellr self-assigned this Nov 7, 2018
@kjellr kjellr requested a review from allancole November 7, 2018 02:37
Copy link
Collaborator

@allancole allancole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is mostly looking good! The main issue I noticed is that some of the custom color selectors fall out of sync between the backend and the frontend.

Backend
image

Frontend
image

Otherwise, looks great 👍

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 7, 2018

🤔 Ah, I bet that's due to adding the .entry wrapper to our block styles. I can fix. 👍

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 7, 2018

@allancole Should be all set now. Mind re-testing?

@allancole allancole merged commit 859b388 into master Nov 8, 2018
@kjellr kjellr deleted the update/margin-spacing branch November 8, 2018 13:19
kjellr added a commit that referenced this pull request Nov 8, 2018
A followup to #502. Closes #236.

Adjusts editor styles between `600px` and `768px` so that the margins are more closely aligned to the front end.

Note that this doesn't make them 100% exact: the editor has about 10px more space on either side. This is because the editor requires more space for the hover/editor controls. We _could_ match these up exactly by adjusting the front end, but I tend to prefer tighter margins in general and I think this is close enough.
kjellr added a commit that referenced this pull request Nov 13, 2018
As a result of our improved margins in #502, the styles here  aren't necessary (and actually end up  breaking things when they're left in).
jeherve pushed a commit to Automattic/jetpack that referenced this pull request Nov 15, 2018
<!--- Provide a general summary of your changes in the Title above -->

Fixes #10585
Fixes #10587

#### Changes proposed in this Pull Request:
<!--- Explain what functional changes your PR includes -->

- [x] Removes margin styles from Related Posts module, Recipe shortcodes and Presentation shortcodes.
- [x] Layout margin styles now come from the theme to prevent layout issues with other plugins. See: WordPress/twentynineteen#502 and WordPress/twentynineteen#534
- [x] Also remove margin styles from Akismet privacy notice since it also gets fixed by the theme here: WordPress/twentynineteen#533

#### Testing instructions:
<!-- Please include detailed testing steps, explaining how to test your change. -->
<!-- Bear in mind that context you working on is not obvious for everyone.  -->
<!-- Adding "simple" configuration steps will help reviewers to get to your PR as quickly as possible. -->
<!-- "Before / After" screenshots can also be very helpful when the change is visual. -->

* Test the Recipe or Presentation shortcode with the latest version of Twenty Nineteen: https://github.com/WordPress/twentynineteen/
* Test the layout against the Akismet notice with the latest version of Twenty Nineteen: https://github.com/WordPress/twentynineteen/

#### Proposed changelog entry for your changes:
<!-- Please do not leave this empty. If no changelog entry needed, state as such. -->

* Fix layout issues in Jetpack modules and shortcodes for Twenty Nineteen theme
jeherve pushed a commit to Automattic/jetpack that referenced this pull request Nov 15, 2018
<!--- Provide a general summary of your changes in the Title above -->

Fixes #10585
Fixes #10587

#### Changes proposed in this Pull Request:
<!--- Explain what functional changes your PR includes -->

- [x] Removes margin styles from Related Posts module, Recipe shortcodes and Presentation shortcodes.
- [x] Layout margin styles now come from the theme to prevent layout issues with other plugins. See: WordPress/twentynineteen#502 and WordPress/twentynineteen#534
- [x] Also remove margin styles from Akismet privacy notice since it also gets fixed by the theme here: WordPress/twentynineteen#533

#### Testing instructions:
<!-- Please include detailed testing steps, explaining how to test your change. -->
<!-- Bear in mind that context you working on is not obvious for everyone.  -->
<!-- Adding "simple" configuration steps will help reviewers to get to your PR as quickly as possible. -->
<!-- "Before / After" screenshots can also be very helpful when the change is visual. -->

* Test the Recipe or Presentation shortcode with the latest version of Twenty Nineteen: https://github.com/WordPress/twentynineteen/
* Test the layout against the Akismet notice with the latest version of Twenty Nineteen: https://github.com/WordPress/twentynineteen/

#### Proposed changelog entry for your changes:
<!-- Please do not leave this empty. If no changelog entry needed, state as such. -->

* Fix layout issues in Jetpack modules and shortcodes for Twenty Nineteen theme
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync up margins, line-length between frontend and editor
2 participants