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 back-to-top link #381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add back-to-top link #381

wants to merge 1 commit into from

Conversation

Ainali
Copy link
Member

@Ainali Ainali commented May 3, 2024

Based on https://codepen.io/chriscoyier/pen/NWmzZXo

Fixes #377

(If #380 is merged, the tests will pass.)

@Ainali Ainali requested a review from Abbe98 May 3, 2024 14:09
@RVA2869
Copy link
Collaborator

RVA2869 commented May 3, 2024

Wouldn't it be better to add the <a href="#top" class="to-top-link">Back to top</a> in layout.html?

@Ainali
Copy link
Member Author

Ainali commented May 3, 2024

I considered that first but chose not to for two reasons:

  1. Most important, it would be outside the <main> tag.
  2. Less important, I think it is really only the country pages that require a lot of scrolling.

@RVA2869
Copy link
Collaborator

RVA2869 commented May 3, 2024

2. Less important, I think it is really only the country pages that require a lot of scrolling.

And the topics pages. ;-)

Copy link
Member

@Abbe98 Abbe98 left a comment

Choose a reason for hiding this comment

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

I would suggest moving the button to the right side, I think that patterns is a little bit more common and it makes sense for (most) mobile users.

  • It currently interacts oddly with the footer, I think this is because it's inside of main(which I don't see why it should be).
  • It currently appears when there is no scrollbar on the page(greenland is a good example)
  • The scroll position at which it first shows seems inconsistent, shouldn't it be there as soon as you are no longer at the top?

@Ainali Ainali added the help wanted Extra attention is needed label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scroll back up button
3 participants