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

Add front end padding to column blocks. #591

Merged
merged 3 commits into from
Nov 16, 2018
Merged

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Nov 14, 2018

Fixes #517.

This PR seeks to add some padding to our alignfull columns so that they match up more closely with the editor.

Appearance in editor:

Note: this is based on the columns appearance seen in the #575 branch. That happens to correct a bug that inserts too-wide margins on the columns block. As a result, it might be helpful to wait until that PR is merged to text/merge this one.

screen shot 2018-11-14 at 11 02 10 am

Current appearance in front end:

screen shot 2018-11-14 at 11 08 08 am

Updated appearance in front end:

Note: This does not line up exactly. But that's mostly because the editor seems to add inconsistent margins on the left and right side of the columns block when it's full-screen. I've filed a Gutenberg bug for that (WordPress/gutenberg#11869), but in the meantime we don't want to replicate that error.

screen shot 2018-11-14 at 11 02 23 am


This also adds padding to smaller screens:

Before:
screen shot 2018-11-14 at 11 30 43 am

After
screen shot 2018-11-14 at 11 31 06 am

@kjellr kjellr added the bug Something isn't working label Nov 14, 2018
@kjellr kjellr requested a review from jasmussen November 14, 2018 16:32
Copy link
Collaborator

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Thank you for working on this.

However since you made this, I have refactored aspects of the columns block in Gutenberg upstream, in WordPress/gutenberg#11904. This PR might not be necessary anymore, not sure. In any case, I tested this PR against that branch, since that's going to be the baseline going forward.

TwentyNineteen master compared with 11904:

Editor:

screenshot 2018-11-15 at 12 06 04

Preview:

screenshot 2018-11-15 at 12 06 10

TwentyNineteen THIS BRANCH compared with 11904:

Editor:

screenshot 2018-11-15 at 12 07 05

This branch:

screenshot 2018-11-15 at 12 07 10

As you can see, comparing to 11904 this branch is actually a small regression.

Sorry for those headaches.

Does it do anything to the frontend appearance of columns?

@jasmussen
Copy link
Collaborator

Worth noting as mentioned also in the upstream PR, the additional left and right padding that's present in the editor is intentional, even though it means the preview is not 1:1 correct: https://github.com/WordPress/gutenberg/compare/improve/columns?expand=1#diff-49ac41bc72e24e343a451fe254801c0fR16

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 15, 2018

Interesting, thanks — I've weighed in on the upstream PR, and will hold off until we see what happens there.

@kjellr
Copy link
Collaborator Author

kjellr commented Nov 16, 2018

I gave this a re-test, and I think it's still a good improvement to include on our end. It only effects the front-end, and just adds a little padding so that text columns don't bump against the side of the viewport. It also happens to make the front-end look a little like the editor, but that's mostly a side effect.

Before:
screen shot 2018-11-16 at 10 19 18 am
screen shot 2018-11-16 at 10 19 34 am

After:
screen shot 2018-11-16 at 10 18 45 am
screen shot 2018-11-16 at 10 19 03 am

(@jasmussen looking at your front-end screenshots above, I wonder if there was a caching issue or something —  I don't see this PR's changes reflected in them).

@jasmussen jasmussen self-requested a review November 16, 2018 15:30
Copy link
Collaborator

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

If these rules do not affect the upstream refactor of columns, ship it! It's totally okay to be opinionated about what "fullwide" for a particular block.

@kjellr kjellr merged commit 45d79e2 into master Nov 16, 2018
@kjellr
Copy link
Collaborator Author

kjellr commented Nov 16, 2018

🎉 Thanks!

@kjellr kjellr deleted the update/column-block-padding branch November 16, 2018 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No left and right margin for fully aligned columns
2 participants