Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upComment tree lines #400
Conversation
talklittle
added some commits
Sep 24, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
xcombelle
Sep 25, 2017
the arrow space is too large at first sight. half of the space should be good
xcombelle
commented
Sep 25, 2017
|
the arrow space is too large at first sight. half of the space should be good |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
colindean
Sep 25, 2017
What if there was a subtle color difference in each level of the thread?
Each top comment gets a different color than the thread before or after it, maybe choosing out of seven or eight colors. Subthreads get a variation on that color or a different color altogether so that it's possible to tell when a subthread has ended based on the color, too.
colindean
commented
Sep 25, 2017
|
What if there was a subtle color difference in each level of the thread? Each top comment gets a different color than the thread before or after it, maybe choosing out of seven or eight colors. Subthreads get a variation on that color or a different color altogether so that it's possible to tell when a subthread has ended based on the color, too. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
talklittle
Sep 25, 2017
Contributor
@xcombelle Good point, I narrowed the indentation back to the original width. See updated screenshot.
@colindean Copying my comment here for completeness
If we agree that color-coding is an improvement, there's still a debate whether the colors should be per top-level comment (vertical rainbow), or the color should indicate the depth of the comment (horizontal rainbow).
So I think it's best to do that in a future pull request after some more thought and experimentation.
|
@xcombelle Good point, I narrowed the indentation back to the original width. See updated screenshot. @colindean Copying my comment here for completeness
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
colindean
commented
Sep 25, 2017
|
Definitely, merge it and then iterate! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jcs
Sep 25, 2017
Contributor
Thanks for tackling this. The mobile/responsive view seems broken though.
|
Thanks for tackling this. The mobile/responsive view seems broken though. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
talklittle
Sep 26, 2017
Contributor
@jcs Thanks for catching that. I've pushed a commit to hide the tree lines on mobile. I think the addition of the top-level folder buttons and tree lines makes the comments section too cramped horizontally.
|
@jcs Thanks for catching that. I've pushed a commit to hide the tree lines on mobile. I think the addition of the top-level folder buttons and tree lines makes the comments section too cramped horizontally. |
jcs
merged commit 9037817
into
lobsters:master
Sep 26, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jcs
Sep 26, 2017
Contributor
I backed this out locally, there's still some bugs to work out.
In this shot you can see at the bottom, the vertical line doesn't continue through the three posts under angersock's. Same in this one missing a line between tedu and SeanTAllen. Also, the top-level collapse box no longer lines up under the '24' at the very top. It would be nice to preserve that alignment if possible.
|
I backed this out locally, there's still some bugs to work out. In this shot you can see at the bottom, the vertical line doesn't continue through the three posts under angersock's. Same in this one missing a line between tedu and SeanTAllen. Also, the top-level collapse box no longer lines up under the '24' at the very top. It would be nice to preserve that alignment if possible. |

talklittle commentedSep 25, 2017
•
edited
Edited 1 time
-
talklittle
edited Sep 25, 2017 (most recent)
ref #189
Based on mockup https://i.imgur.com/677PsUV.png from #233
Current result (tested on Firefox / Ubuntu desktop only):
Updated with narrower indentation
Previous screenshot