Skip to content
This repository has been archived by the owner on Sep 22, 2021. It is now read-only.

Added anchor to comments #824

Merged
merged 13 commits into from Jun 4, 2020
Merged

Added anchor to comments #824

merged 13 commits into from Jun 4, 2020

Conversation

niklabh
Copy link
Contributor

@niklabh niklabh commented May 31, 2020

Closes: #776

  • anchors on each comment
  • an additional #end anchor on the last one
  • link directly to comments in the "new comment posted" emails
  • share comments with others
  • go straight to the end of a thread by just adding #end to the end of a thread's URL
  • add a link "Go to bottom" to the top of the thread which will take a user to #end, i.e. the last comment.

chrome-capture (6)

@niklabh niklabh requested review from Tbaut and erler and removed request for Tbaut May 31, 2020 17:49
@niklabh niklabh requested a review from Tbaut May 31, 2020 17:51
@erler
Copy link
Collaborator

erler commented May 31, 2020

One other thing worth considering maybe would be to have the comment timestamp as a link for the comment (similar to Github and others). Though this might make the CreationLabel a bit too convoluted.

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

I haven't tested it yet, but there are many placed where we don't follow React best practices. It'd be great if we can adapt these getElementById to useRef instead.

front-end/src/components/Comment/Comment.tsx Outdated Show resolved Hide resolved
front-end/src/components/Post/Post.tsx Outdated Show resolved Hide resolved
@niklabh
Copy link
Contributor Author

niklabh commented Jun 4, 2020

I haven't tested it yet, but there are many placed where we don't follow React best practices. It'd be great if we can adapt these getElementById to useRef instead.

done

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Nice thanks it's cool now.
I think this "back to top" is totally invisible, and that the "go to bottom" is also not placed where it probably should (all the action button next to it relate to the post, this is unrelated).

but overall it's bringing a lot of value so let's merge. I'll create an issue for the rest. (edit: #842)

auth-server/src/utils/getPostCommentLink.ts Outdated Show resolved Hide resolved
@niklabh
Copy link
Contributor Author

niklabh commented Jun 4, 2020

Nice thanks it's cool now.
I think this "back to top" is totally invisible, and that the "go to bottom" is also not placed where it probably should (all the action button next to it relate to the post, this is unrelated).

but overall it's bringing a lot of value so let's merge. I'll create an issue for the rest. (edit: #842)

I tried to add a circular button but couldn't find a good place for it

@Tbaut
Copy link
Collaborator

Tbaut commented Jun 4, 2020

I'm not convinced we need a go to bottom tbh now that we have direct links. Back to top yes, and then it can be a button just like you had, but at the bottom. That would surely work.

@Tbaut Tbaut merged commit 0cf1712 into master Jun 4, 2020
@Tbaut Tbaut deleted the niklabh-anchor-comments branch June 4, 2020 22:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add anchors to comments
3 participants