Skip to content

Add the back-to-top button to the single post view#7729

Closed
denschub wants to merge 2 commits intodiaspora:developfrom
denschub:7727-add-backtotop-to-spv
Closed

Add the back-to-top button to the single post view#7729
denschub wants to merge 2 commits intodiaspora:developfrom
denschub:7727-add-backtotop-to-spv

Conversation

@denschub
Copy link
Copy Markdown
Member

@denschub denschub commented Mar 3, 2018

This would fix #7727.

Note that I added the back-to-top thingy on the same level as the content container, not inside it. This needs to be done, since we overwrite the contents of the container entirely, and that would override the link as well. Since the button is position: fixed; it won't matter anyway.

@denschub denschub added this to the 0.7.4.0 milestone Mar 3, 2018
@SuperTux88
Copy link
Copy Markdown
Member

I think it makes sense to add the back-to-top thingy in the application template, so we have it on every site (for example on the conversations, where it's currently missing too) and it is also the same on every site and we don't have it on 5 different places in 5 different templates.

But I think it would be cool to make it a bit more "intelligent" then, for example disable it completelly when the page is already on the top or only activate it at all when the site is more than 1.5 or 2 times the window-size (currently it appears when you hover in the corner even when you are at the top, which could in the worst case block other buttons). Currently it appears when you scroll down 1000px, otherwise it appears when you hover on it, we could also make this 1000px a must and remove the hover activation. Something like that, I don't know which is the easiest to implement at the moment, and I don't want to refactor everything at the moment, so just if something like this is easily doable with the current frontend :)

@denschub
Copy link
Copy Markdown
Member Author

denschub commented Mar 6, 2018

Yeah, that's actually a good call. We added the back-to-top thingy to some pages before (#4191, for example), and there probably will be more PRs in the future requesting it to be added even further, like in the conversations, as you mentioned. So I agree that adding it globally is probably a good idea, especially since it shouldn't do any harm. Thanks for the suggestion!

I refactored the widget so it does not show up on how on hover if the page is scrolled to less than 1000px scrollTop. The element is still there, but invisible and click-through, so it doesn't do any harm. Adding the element only when needed (or using display: none;) would cause a font reflow after the element is already visible, which can cause visual flicker, so that's the best solution here.

I also fixed two bugs drive-by:

  1. When focusing the element by other means (keyboard navigation, for example), you could see the default blue-underlined link style.
  2. When an user scrolled just below the threshold and clicked the button within 250ms, the scrollUp event gets throttled away by underscore, and we show the button forever. I fixed that by explicitly re-checking the visibility after the animation is done.

… is done

If a user scrolls just below the 1000px threshold and clicks the back-to-top button within 250ms, we'll never hide the button as the scroll event gets throttled away. That's rather inconvenient.
Copy link
Copy Markdown
Member

@SuperTux88 SuperTux88 left a comment

Choose a reason for hiding this comment

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

🍪

@SuperTux88 SuperTux88 closed this in 3643612 Mar 6, 2018
@SuperTux88
Copy link
Copy Markdown
Member

Merged as 8e88f4b and 3643612, thank you 🍪

@denschub denschub deleted the 7727-add-backtotop-to-spv branch March 7, 2018 00:21
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.

Add back-to-top button to single-post-view

2 participants