Cleaned up aside formatting #326
Conversation
|
👍🏻 presuming everything ends up looking the same |
|
Two visual changes:
|
| margin-bottom: 1em; | ||
| border-left: $aside_note_border; | ||
| .aside, .Swift { | ||
| padding: 0.4em 0.8em; |
There was a problem hiding this comment.
why the change from px to em? I prefer the spacing from before.
before
after
There was a problem hiding this comment.
Hmm, the “before” is weirdly asymmetric on my local setup, and my change fixed it. Both of your screenshots look wrong to my eyes. Will investigate when I have a free minute….
There was a problem hiding this comment.
I agree that the "before" version isn't perfect, but better than what I generated from this PR.
There was a problem hiding this comment.
Right. What I’m saying is that they both look quite different on my machine, before as well as after, and I need to figure out why — maybe local changes….
There was a problem hiding this comment.
Here’s what I see after the change:
Note the symmetry above & below. Clearly not what you’re getting!
There was a problem hiding this comment.
That looks quite nice! Maybe I'm generating this wrong? Seems unlikely though, since I ran rake rebuild_integration_fixtures and the new CSS classes were being resolved.
There was a problem hiding this comment.
Odds are good that it’s something about my hacked-up local setup. Investigating, and I’ll add a commit to the PR when I have it figured out.
There was a problem hiding this comment.
Aha! Neither your mistake nor mine. Sometimes the aside content is inside a <p> tag, and sometimes it isn’t. You were seeing a <p> case, and I was seeing a bare text case. Just pushed a fix to make it consistent.
|
Other than the margin changes, 👍 thanks! PS: I caught this as I was rebuilding the integration fixtures, and I'm happy to do it again once this is ready to go! |
|
OK, I think this is ready to go. Take a look at the new output and see how it looks to you now, @jpsim. Do I need to have Xcode 6.x and 7.0 installed, as well as 7.1? I experimented with rebuilding the integration fixtures, but get errors about missing compilers for Swift 1.2 and 2.0. |
|
Looks good now! You can point the integration specs submodule to realm/jazzy-integration-specs@fff44b9 now.
Currently compilers for Swift 1.2 and Swift 2.0 are required to run the full test suite. |
pcantrell
force-pushed
the
aside-formatting-cleanup
branch
from
ebffcaa to
9981575
Compare
|
Repointed integration specs and merged. First time doing that, so please LMK if I did anything wrong. |
pcantrell
force-pushed
the
aside-formatting-cleanup
branch
from
b7d7e0d to
12e0912
Compare
|
Hurray! |
This addresses #325, at least in part.
I’m unsure how to update the integration specs. (My usual approach to git submodules is to hide until they go away.) Do I need to create a separate PR on the specs repo…? @jpsim, can you lend a hand with this if I make very sad puppy eyes?