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

Alternative approach for comments-time-machine-links #2618

Closed
fregante opened this issue Dec 11, 2019 · 2 comments · Fixed by #3135
Closed

Alternative approach for comments-time-machine-links #2618

fregante opened this issue Dec 11, 2019 · 2 comments · Fixed by #3135
Assignees
Labels
change request help wanted small Issues that new contributors can pick up

Comments

@fregante
Copy link
Member

fregante commented Dec 11, 2019

The timemachine link is useful but I never click it unless I really need that version, and most of the time I don't know this until I click the main link.

I suggest dropping the icon (closes #2131) and, instead, waiting for the user to click the link.

On the resulting page:

  • if the linked file still exists, add the timemachine permalink near branch-buttons:
    linked
  • if the linked file is gone, add the timemachine link somewhere on the 404 page

This also makes it easier to enable #1953 because we don't have to query each link on the page before adding the link: we just query the current page.


For this to be possible, instead of adding an icon to these links, we should add a ?linked-time=${timestamp} parameter so this information is available on the linked page.

Test

@sindresorhus
Copy link
Member

Sounds good 👍

@fregante
Copy link
Member Author

fregante commented Feb 28, 2020

Here's something easy to add in both cases (existing file or 404 page) and easy to understand:

Example code:

<div id="js-flash-container">
  <div class="flash flash-full flash-notice">
	  <div class="container-lg px-2">
	    <button class="flash-close js-flash-close" type="button" aria-label="Dismiss this message">{xIcon()}</button>
		You can also <a href="">view this file as it appeared at the time of the comment</a> (4 days ago)
		</div>
	</div>
</div>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change request help wanted small Issues that new contributors can pick up
Development

Successfully merging a pull request may close this issue.

2 participants