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

[UI] Fix whitespace being applied to div instead of p #9968

Merged
merged 2 commits into from Feb 5, 2019

Conversation

Projects
None yet
2 participants
@trwnh
Copy link
Contributor

trwnh commented Feb 4, 2019

Before: image

After:
image

white-space: pre-wrap is an unnecessary rule, and removing it fixes the issue without introducing any noticeable side effects. The large gap was occurring mainly in posts from Pleroma, which would add the white space and also add margin-bottom: 20px, which led to effectively two line breaks for every new p element.

@Gargron

This comment has been minimized.

Copy link
Member

Gargron commented Feb 4, 2019

Did you check how the ascii art posts look?

@trwnh

This comment has been minimized.

Copy link
Contributor Author

trwnh commented Feb 5, 2019

Hmm, I had not. I guess adjusting white-space is the wrong strategy, then. But I can't exactly figure out why this seems to be happening -- there seems to be an issue with line height somehow, as this element is exactly 2 lines high despite only containing a 1-line p:

image

image

Disabling the white-space rule causes the div to take up 1 line again, but with the side effect of ruining ascii art posts.

@Gargron

This comment has been minimized.

Copy link
Member

Gargron commented Feb 5, 2019

My guess is that there is an actual linebreak in the source of the post after the </p> tag.

I'm guessing ascii art never worked in Pleroma?

@trwnh

This comment has been minimized.

Copy link
Contributor Author

trwnh commented Feb 5, 2019

I have no idea if ascii art posts ever worked in Pleroma, as that's up to their styling. However, there is no actual line break at all in the source. The div wraps the p and nothing else -- no <br> at all. However, I have absolutely no idea why it's not happening with posts from Mastodon. It's possible that Mastodon posts include extra markup that Pleroma posts do not.

However, the issue with ascii art posts is solved by instead applying the white-space rule to the p instead of its containing div. The margin-bottom preserves spacing between p elements, while the p itself is free to include whitespace that isn't collapsed. Updated the PR to preserve ascii art post styling while still fixing the double-line issue.

@trwnh

This comment has been minimized.

Copy link
Contributor Author

trwnh commented Feb 5, 2019

A better demonstration of why adding white-space: pre-wrap to the div is problematic: it means you can't use whitespace in your source code at all.

Mastodon's source:
image

Pleroma's source:
image

Removing white-space from inside the div yields this:
image

Adding more white-space yields this:
image

So applying the rule to the div means that this will take multiple lines:

<div>
    <p>test</p>
</div>

And so this is the only way to get a single line:

<div><p>test</p><div>

Thus, the white-space rule should be applied to the p where it will actually be used, and not to the parent div which should not care about whitespace.

@trwnh trwnh changed the title [UI] Fix large line breaks [UI] Fix whitespace being applied to div instead of p Feb 5, 2019

@Gargron

Gargron approved these changes Feb 5, 2019

@Gargron Gargron merged commit 76d4147 into tootsuite:master Feb 5, 2019

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.6 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details

@trwnh trwnh deleted the trwnh:patch-1 branch Feb 5, 2019

Gargron added a commit that referenced this pull request Feb 17, 2019

[UI] Fix whitespace being applied to div instead of p (#9968)
* fix large line breaks

* fix ascii art posts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.