Add page to view specific entry comments #648
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
change show nested to use current view setting
change cache lookup to be by comment id and not the root comment id
update template based on improvements made to others
add event listener to mark notifications as read when viewed
highlight comment the url is for
handle private content rename parent to comment to be more accurate
e-five256
commented
Apr 7, 2024
Comment on lines
+16
to
+22
| <style> | ||
| #entry-comment-{{comment.id}} { | ||
| border-top: var(--kbin-alert-danger-border); | ||
| border-bottom: var(--kbin-alert-danger-border); | ||
| border-right: var(--kbin-alert-danger-border); | ||
| } | ||
| </style> |
There was a problem hiding this comment.
this is a bit hacky where; I can't modify the css in the nested comments (like adding highlighted to the comment class list) as it's cached html, so instead this sets the css for the comment that is being viewed to style it differently. It could also be done via js but this doesn't seem that bad, but I could be wrong
nobodyatroot
approved these changes
Apr 25, 2024
There was a problem hiding this comment.
I don't see a good reason to hold this up, speak now or forever hold your peace @asdfzdfj @BentiGorlich @melroy89
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This creates a new view that is specifically for viewing an entry comment.
Problems attempting to be solved:
EntryCreateController) requires#[IsGranted('ROLE_USER')]The page loaded here shows an entire comment chain from root, including all its replies. It proved difficult to trim off comments from either direction as everything under a root is cached html so any changes would reflect how it's cached (there's more details of my issues collapsed at the bottom of the PR)
❌ only clear notifications related to comment, not entire entry (out of scope imo)
other theme examples
outline is less noticeable in light themes but the browser view still does jump down to the comment using url fragment so that helps (I'm manually scrolling up for screenshots)
challenges faced
Open issues:I've been stuck on one problem for a very long time:
I imagine people would prefer this to show replies in at least one direction if not both. Specifically, show comments that replied to this or the parent that it was replying to.
I was having a lot of trouble with that, and doing the always show timestamp PR helped me understand why, the replies to a top level entry are all pulled in via
entry_comments_nestedcomponent which is heavily cached by the root parent id. If I tried to show the nested replies, instead it would duplicate the entire thread from top to bottom including the entry comment you were looking at...And if you tried to trim comments, if you loaded the comment view first and went to the full thread, those comments would be missing since it was cached together.
If you tried to change the cache key so everything was cached differently, there would still be oddities (level of comment chain for instance)
As commented, viewing an entry comment marks all notifications for that entry as viewed. This isn't different than how it's always been, but is more noticeable now that you might see less of the comments.
Fixes #406