Skip to content

Thread collapsing redux#2349

Merged
tilgovi merged 4 commits into
masterfrom
thread-collapsing-redux
Jul 7, 2015
Merged

Thread collapsing redux#2349
tilgovi merged 4 commits into
masterfrom
thread-collapsing-redux

Conversation

@JakeHartnell
Copy link
Copy Markdown
Contributor

This reintroduces the thread collapsing improvements as described in #2337.

It appears that commit cdddb16 broke thread collapsing by hiding the bodies of top level annotations. This has been fixed by a small adjustment to annotations.scss.

@landscape-bot
Copy link
Copy Markdown

Code Health
Code quality remained the same when pulling 5653095 on hypothesis:thread-collapsing-redux into 561544e on hypothesis:master.

JakeHartnell and others added 3 commits July 6, 2015 15:44
Make the tags section use .annotation-body (because tags are a form of
body) and put the form actions, reply count, and license messages into
the footer so that these collpasing properly and uniformly.
Since there is no username, collapsing a draft when not signed in
had been causing the annotation to disappear completely. Now, there
is a clear prompt that holds the space.
@tilgovi
Copy link
Copy Markdown
Contributor

tilgovi commented Jul 7, 2015

Cool. I'm poking at this and found a couple rough edges. Collapsing annotations while they are being edited results in awkwardly having save/cancel buttons without the annotation body. Hiding these results in a small, excess margin on annotations being edited. Hiding this results in new drafts disappearing completely when collapsed if the user is not logged in. I've mostly addressed all these and I'll push in the morning for your review.

@tilgovi tilgovi force-pushed the thread-collapsing-redux branch from 5653095 to d30a6e4 Compare July 7, 2015 20:33
tilgovi added a commit that referenced this pull request Jul 7, 2015
@tilgovi tilgovi merged commit 65b9c3c into master Jul 7, 2015
@tilgovi tilgovi deleted the thread-collapsing-redux branch July 7, 2015 20:34
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.

3 participants