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

Remove block top/bottom padding #399

Merged
merged 7 commits into from
Dec 19, 2018
Merged

Remove block top/bottom padding #399

merged 7 commits into from
Dec 19, 2018

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Dec 18, 2018

Fixes #393

Removes the top/bottom padding on block-container as it was adding an unneeded extra space.

Before:
screen shot 2018-12-18 at 11 57 55

After:
screen shot 2018-12-18 at 11 56 39

Before (WPAndroid):
screen shot 2018-12-18 at 12 02 18

After (WPAndroid):
screen shot 2018-12-18 at 12 03 29

Before (demo app on iOS):
screen shot 2018-12-18 at 12 13 50

After (demo app on iOS):

screen shot 2018-12-18 at 12 19 03

@mzorz mzorz added the Blocks label Dec 18, 2018
@mzorz mzorz added this to the Alpha milestone Dec 18, 2018
@iamthomasbishop
Copy link
Contributor

Thanks for pinging for review. Unsupported blocks should still maintain that top/bottom padding, so how it was in the "before" examples was proper. Example:

image

Note: each block, regardless of type, has the same padding: 12pt top/bottom, 16pt left/right.

I think we should also think about implementing the styling shown here, but that's for a separate issue.

@mzorz
Copy link
Contributor Author

mzorz commented Dec 18, 2018

Updated in 9f21f87, where it's been updated to match that 12dp value for top/bottom for unsupported blocks only:

Before

screen shot 2018-12-18 at 12 58 09

After

screen shot 2018-12-18 at 12 58 30

does it look good now @iamthomasbishop ?

@mzorz
Copy link
Contributor Author

mzorz commented Dec 18, 2018

Note: each block, regardless of type, has the same padding: 12pt top/bottom, 16pt left/right.

oops! this is fixed then in a1bdd09

results:

screen shot 2018-12-18 at 13 09 35

@mzorz
Copy link
Contributor Author

mzorz commented Dec 18, 2018

ok - ready for another pass @iamthomasbishop , implemented the specs described in #393 (comment) in 81d9704, this is how it looks like (before/after):

screen shot 2018-12-18 at 15 03 48

@mzorz
Copy link
Contributor Author

mzorz commented Dec 19, 2018

Merging as agreed

@mzorz mzorz merged commit f87f75a into master Dec 19, 2018
@mzorz mzorz deleted the issue/393-bottom-margin branch December 19, 2018 19:33
@iamthomasbishop
Copy link
Contributor

As discussed in dm, the block inner body padding/margin stuff looks good here, so feel free to 🚢!

We can handle this in a separate issue, but the spacings on the inline toolbar (up/down/trash) needs to be slightly tweaked to include a slight (2pt) margin on both left/right edges of the toolbar. This helps to visually align the block content a little better.

image

@mzorz mzorz mentioned this pull request Dec 19, 2018
@mzorz
Copy link
Contributor Author

mzorz commented Dec 19, 2018

no worries, opened issue here #409 - let's follow discussion there or on upcoming PR

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

Successfully merging this pull request may close these issues.

2 participants