From 4069c33e58397fbf6971f98a399d71d9d36eeba8 Mon Sep 17 00:00:00 2001 From: Chad Birch Date: Tue, 28 Aug 2018 18:31:36 -0600 Subject: [PATCH] Collapse old comments when re-visiting a topic For users that have the "mark new comments" feature enabled, this will collapse old comments when they re-visit a topic that has new ones. It involves adding a new "individual collapse" style that only collapses a single comment and doesn't also hide all of its replies. New comments and their direct parents will stay uncollapsed, and all other comments in a path up to the root will be individually collapsed. Any branches with no expanded comments will be fully collapsed. We should probably add an indicator for how many comments are in a collapsed chain so that we can distinguish between individually collapsed ones and larger collapsed chains. --- tildes/scss/modules/_comment.scss | 8 +++ .../behaviors/comment-collapse-all-button.js | 8 +++ .../js/behaviors/comment-collapse-button.js | 8 ++- .../js/behaviors/comment-expand-all-button.js | 2 +- tildes/tildes/jinja.py | 1 - tildes/tildes/models/comment/comment_tree.py | 72 +++++++++++++++++-- .../tildes/templates/macros/comments.jinja2 | 10 ++- tildes/tildes/views/topic.py | 8 ++- 8 files changed, 106 insertions(+), 11 deletions(-) diff --git a/tildes/scss/modules/_comment.scss b/tildes/scss/modules/_comment.scss index cf5d5c0..93db50b 100644 --- a/tildes/scss/modules/_comment.scss +++ b/tildes/scss/modules/_comment.scss @@ -142,6 +142,14 @@ } } +// uses @extend to only collapse everything inside the collapsed comment itself and +// not its replies +.is-comment-collapsed-individual { + & > .comment-itself { + @extend .is-comment-collapsed; + } +} + .is-comment-deleted, .is-comment-removed { font-size: 0.7rem; font-style: italic; diff --git a/tildes/static/js/behaviors/comment-collapse-all-button.js b/tildes/static/js/behaviors/comment-collapse-all-button.js index 64827a5..d90190b 100644 --- a/tildes/static/js/behaviors/comment-collapse-all-button.js +++ b/tildes/static/js/behaviors/comment-collapse-all-button.js @@ -1,5 +1,13 @@ $.onmount('[data-js-comment-collapse-all-button]', function() { $(this).click(function(event) { + // first uncollapse any individually collapsed comments + $('.is-comment-collapsed-individual').each( + function(idx, child) { + $(child).find( + '[data-js-comment-collapse-button]:first').trigger('click'); + }); + + // then collapse all first-level replies $('.comment[data-comment-depth="1"]:not(.is-comment-collapsed)').each( function(idx, child) { $(child).find( diff --git a/tildes/static/js/behaviors/comment-collapse-button.js b/tildes/static/js/behaviors/comment-collapse-button.js index 4edd188..f277838 100644 --- a/tildes/static/js/behaviors/comment-collapse-button.js +++ b/tildes/static/js/behaviors/comment-collapse-button.js @@ -3,7 +3,13 @@ $.onmount('[data-js-comment-collapse-button]', function() { $this = $(this); $comment = $this.closest('.comment'); - $comment.toggleClass('is-comment-collapsed'); + // if the comment is individually collapsed, just remove that class, + // otherwise toggle the collapsed state + if ($comment.hasClass('is-comment-collapsed-individual')) { + $comment.removeClass('is-comment-collapsed-individual'); + } else { + $comment.toggleClass('is-comment-collapsed'); + } if ($comment.hasClass('is-comment-collapsed')) { $this.text('+'); diff --git a/tildes/static/js/behaviors/comment-expand-all-button.js b/tildes/static/js/behaviors/comment-expand-all-button.js index f870139..53c5ce5 100644 --- a/tildes/static/js/behaviors/comment-expand-all-button.js +++ b/tildes/static/js/behaviors/comment-expand-all-button.js @@ -1,6 +1,6 @@ $.onmount('[data-js-comment-expand-all-button]', function() { $(this).click(function(event) { - $('.comment.is-comment-collapsed').each( + $('.is-comment-collapsed, .is-comment-collapsed-individual').each( function(idx, child) { $(child).find( '[data-js-comment-collapse-button]:first').trigger('click'); diff --git a/tildes/tildes/jinja.py b/tildes/tildes/jinja.py index 227404a..1b5f3ed 100644 --- a/tildes/tildes/jinja.py +++ b/tildes/tildes/jinja.py @@ -31,7 +31,6 @@ def includeme(config: Configurator) -> None: settings["jinja2.lstrip_blocks"] = True settings["jinja2.trim_blocks"] = True - settings["jinja2.undefined"] = "strict" # add custom jinja filters settings["jinja2.filters"] = {"ago": descriptive_timedelta} diff --git a/tildes/tildes/models/comment/comment_tree.py b/tildes/tildes/models/comment/comment_tree.py index b04fdd7..b4299f6 100644 --- a/tildes/tildes/models/comment/comment_tree.py +++ b/tildes/tildes/models/comment/comment_tree.py @@ -1,5 +1,6 @@ """Contains the CommentTree class.""" +from datetime import datetime from typing import Iterator, List, Optional, Sequence from prometheus_client import Histogram @@ -16,6 +17,7 @@ class CommentTree: - `replies`: the list of all immediate children to that comment - `has_visible_descendant`: whether the comment has any visible descendants (if not, it can be pruned from the tree) + - `collapsed_state`: whether to display this comment in a collapsed state """ def __init__(self, comments: Sequence[Comment], sort: CommentSortOption) -> None: @@ -27,6 +29,11 @@ class CommentTree: # ensure that parent comments are always processed first self.comments = sorted(comments, key=lambda c: c.created_time) + for comment in self.comments: + comment.collapsed_state = None + + self.comments_by_id = {comment.comment_id: comment for comment in comments} + # if there aren't any comments, we can just bail out here if not self.comments: return @@ -45,15 +52,12 @@ class CommentTree: def _build_tree(self) -> None: """Build the initial tree from the flat list of Comments.""" - comments_by_id = {} - for comment in self.comments: comment.replies = [] comment.has_visible_descendant = False - comments_by_id[comment.comment_id] = comment if comment.parent_comment_id: - parent_comment = comments_by_id[comment.parent_comment_id] + parent_comment = self.comments_by_id[comment.parent_comment_id] parent_comment.replies.append(comment) # if this comment isn't deleted, work back up towards the root, @@ -66,7 +70,7 @@ class CommentTree: break next_parent_id = parent_comment.parent_comment_id - parent_comment = comments_by_id[next_parent_id] + parent_comment = self.comments_by_id[next_parent_id] else: self.tree.append(comment) @@ -156,3 +160,61 @@ class CommentTree: num_comments_range=num_comments_range, order=self.sort.name, ) + + def collapse_old_comments(self, threshold: datetime) -> None: + """Collapse old comments in the tree. + + Any comments newer than the threshold will be uncollapsed, along with their + direct parents. All comments with any uncollapsed descendant will be collapsed + individually. Branches with no uncollapsed comments will be collapsed fully. + """ + for comment in reversed(self.comments): + # as soon as we reach an old comment, we can stop + if comment.created_time <= threshold: + break + + if comment.is_deleted or comment.is_removed: + continue + + # uncollapse the comment + comment.collapsed_state = "uncollapsed" + + # fetch its direct parent and uncollapse it as well + parent_comment: Optional[Comment] = None + if comment.parent_comment_id: + parent_comment = self.comments_by_id[comment.parent_comment_id] + parent_comment.collapsed_state = "uncollapsed" + + # then follow parents to the root, individually collapsing them all + while parent_comment: + if not parent_comment.collapsed_state: + parent_comment.collapsed_state = "individual" + + if parent_comment.parent_comment_id: + parent_comment = self.comments_by_id[ + parent_comment.parent_comment_id + ] + else: + parent_comment = None + + self._finalize_collapsing() + + def _finalize_collapsing(self) -> None: + """Finish collapsing that was done partially by a different method.""" + # if all the comments would end up collapsed, leave them all uncollapsed + if all([comment.collapsed_state is None for comment in self.comments]): + return + + # go over each top-level comment and go into each branch depth-first. Any + # comment that still has its state as None can be fully collapsed (and we can + # stop looking down that branch) + def recursively_collapse(comment: Comment) -> None: + if not comment.collapsed_state: + comment.collapsed_state = "full" + return + + for reply in comment.replies: + recursively_collapse(reply) + + for comment in self.tree: + recursively_collapse(comment) diff --git a/tildes/tildes/templates/macros/comments.jinja2 b/tildes/tildes/templates/macros/comments.jinja2 index 52dfe3d..6e6d5ea 100644 --- a/tildes/tildes/templates/macros/comments.jinja2 +++ b/tildes/tildes/templates/macros/comments.jinja2 @@ -35,7 +35,9 @@ {% macro render_comment_contents(comment, is_individual_comment=False) %}
- + {% if request.has_permission('view', comment) %} {{ username_linked(comment.user.username) }} @@ -206,6 +208,12 @@ {% elif request.has_permission('view_author', comment.topic) and comment.user == comment.topic.user %} {% do classes.append('is-comment-by-op') %} {% endif %} + + {% if comment.collapsed_state == "full" %} + {% do classes.append("is-comment-collapsed") %} + {% elif comment.collapsed_state == "individual" %} + {% do classes.append("is-comment-collapsed-individual") %} + {% endif %} {% endif %} {{ classes|join(' ') }} diff --git a/tildes/tildes/views/topic.py b/tildes/tildes/views/topic.py index 7f2e53f..8cf9d4d 100644 --- a/tildes/tildes/views/topic.py +++ b/tildes/tildes/views/topic.py @@ -272,13 +272,17 @@ def get_topic(request: Request, comment_order: CommentSortOption) -> dict: .all() ) - # if the feature's enabled, update their last visit to this topic + # if the user has the "mark new comments" feature enabled if request.user and request.user.track_comment_visits: + # update their last visit time for this topic statement = TopicVisit.generate_insert_statement(request.user, topic) - request.db_session.execute(statement) mark_changed(request.db_session) + # collapse old comments if the user has a previous visit to the topic + if topic.last_visit_time: + tree.collapse_old_comments(topic.last_visit_time) + return { "topic": topic, "log": log,