Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add permalinks to changeset comments #4789

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gy-mate
Copy link

@gy-mate gy-mate commented May 14, 2024

Fixes #2796

@@ -33,7 +33,7 @@
<li id="c<%= comment.id %>">
<small class='text-muted'>
<%= t comment.visible ? ".comment_by_html" : ".hidden_comment_by_html",
:time_ago => friendly_date_ago(comment.created_at),
:time_ago => link_to(friendly_date_ago(comment.created_at), "https://www.openstreetmap.org/changeset/#{@changeset.id}##{comment.id}"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:time_ago => link_to(friendly_date_ago(comment.created_at), "https://www.openstreetmap.org/changeset/#{@changeset.id}##{comment.id}"),
:time_ago => link_to(friendly_date_ago(comment.created_at), "/changeset/#{@changeset.id}##{comment.id}"),

But there are probably more rail-ish ways to create that url....

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can generally get the path to an object using *_path helpers, e.g. changeset_path. So changeset_path(changeset) will link to the changeset. If you need an anchor (i.e. a #something in the url) then have a look through the views for other places that use the word "anchor", like for diary entry comments.

You will also find that in some circumstances you can simplify the link_to("foo", changeset_path(changeset)) further, since (most of the time) rails knows how to build a path for an object, e.g. link_to("foo", changeset)`. You can check other places in the existing codebase to learn what works.

@AntonKhorev
Copy link
Contributor

There was a previous attempt #4134. It had the advantage of NOT adding link to the time, in case clicking the time is supposed to do something else like #4361.

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.

Changeset comments have individual links, but no button to grab them with
4 participants