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

Sets limit to comment indents | 5th level #1090

Merged
merged 2 commits into from
Nov 20, 2017

Conversation

robert9g
Copy link
Contributor

Fixes #1088 .

Changes:

  • added CSS class Comment__replies--never-indent
  • put that class in comments starting with 5th level
  • removed @media @small from .Comment__text so that comment avatars would show up after 5th level
  • updated the hardcoded .Comment__replies--no-indent margin to use the @comment-text-offset-small var

Observations:
I know that there were issues between the comment avatars (#883) and the code blocks (#771), and removing overflow:hidden; was needed to properly render code block. But this should be fixed because we calculate the width for each .Comment__text element.

I tested on these pages:

  • has a lot of code comments: /test/@hellosteem/5ihr1p-test#@hellosteem/re-hellosteem-5ihr1p-test-20171018t132820462z
  • has a lot of replies: /cesky/@bucipuci/narocna-kapca-challenge-captcha
  • has the @minnowpond comment with the code element in it: /philosophy/@michellecarter/what-is-the-highest-power

@bonustrack bonustrack temporarily deployed to busy-nd-pr-1090 November 17, 2017 19:15 Inactive
Copy link
Contributor

@jm90m jm90m left a comment

Choose a reason for hiding this comment

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

working well for me, maybe @bonustrack can take a final look since he seems to catch lots of random cases

Copy link
Contributor

@bonustrack bonustrack left a comment

Choose a reason for hiding this comment

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

Looking good

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

Successfully merging this pull request may close these issues.

4 participants