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

[client] When scrolling to a linked comment, it should have some highlight #838

Closed
BBaoVanC opened this issue Apr 23, 2022 · 4 comments · Fixed by #845
Closed

[client] When scrolling to a linked comment, it should have some highlight #838

BBaoVanC opened this issue Apr 23, 2022 · 4 comments · Fixed by #845
Labels
client (Javascript) client code and CSS
Milestone

Comments

@BBaoVanC
Copy link
Contributor

When using a link such as https://example.com/post-1/#isso-4 for example, the comment that is being linked to should flash a couple times to make it identifiable from other nearby comments.

@ix5 ix5 added needs-contributor Someone needs to implement this. Help wanted! client (Javascript) client code and CSS needs-decision Architectural/Behavioral decision by maintainers needed labels Apr 23, 2022
@ix5 ix5 changed the title When scrolling to a linked comment, it should flash a couple times to identify it [client] When scrolling to a linked comment, it should flash a couple times to identify it Apr 23, 2022
@ix5 ix5 added this to the 0.13 milestone Apr 23, 2022
@ix5
Copy link
Member

ix5 commented Apr 23, 2022

That's a bit overkill imo. Putting a slight border around the comment which fades away after a few seconds sounds okay though. Happy to hear any other opinions and links to / images of concrete examples.

Might be implemented via css :focus selectors?

@ix5 ix5 added the good-first-issue Help welcome! label Apr 23, 2022
@BBaoVanC BBaoVanC changed the title [client] When scrolling to a linked comment, it should flash a couple times to identify it [client] When scrolling to a linked comment, it should have some highlight Apr 23, 2022
@BBaoVanC
Copy link
Contributor Author

That's a bit overkill imo. Putting a slight border around the comment which fades away after a few seconds sounds okay though. Happy to hear any other opinions and links to / images of concrete examples.

Might be implemented via css :focus selectors?

It looks like this already mostly works using the CSS :target selector. Here's an example that mostly works with an unmodified Isso instance.

.isso-comment:target {
    animation: target-fade 5s 1;
}

@keyframes target-fade {
    0% { background-color: #eee; }
    100% { background-color: transparent; }
}

A couple problems currently but it should be easy to fix:

  1. it doesn't show on the initial page load. It looks like it only shows if you click a comment to link to it when the comment section is already loaded. I'm not sure about how to fix this
  2. needs a little padding and potentially maybe some rounded corners or something so it looks a bit better.

I'm not familiar with JS so I don't really know how to do this properly, especially the first issue I mentioned.

This is how it looks on my site with that CSS (only I modified the color a bit). It fades out after 5 seconds: initial color before fade

With my example, it fades out after 5 seconds. It looks like browser support is fine too, according to MDN. animation support table on MDN

@ix5
Copy link
Member

ix5 commented Apr 23, 2022

Thank you @BBaoVanC for a great and detailed comment. This is what makes the lives of maintainers much easier!

The suggestion you posted looks pretty good already. Maybe you could open a Pull Request to allow others to weigh in on the implementation side? Might be a good idea to link to a live version, either on your website or via some service like jsfiddle.

@ix5
Copy link
Member

ix5 commented Apr 24, 2022

Tracked in linked PR: #845

@ix5 ix5 closed this as completed Apr 24, 2022
@ix5 ix5 removed needs-contributor Someone needs to implement this. Help wanted! needs-decision Architectural/Behavioral decision by maintainers needed good-first-issue Help welcome! labels Apr 24, 2022
@BBaoVanC BBaoVanC moved this to In Progress in Isso Todo May 14, 2022
@BBaoVanC BBaoVanC removed this from Isso Todo May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client (Javascript) client code and CSS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants