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

Really strange scrolling issue when paging with firefox #562

Closed
tswfi opened this issue Nov 20, 2023 · 2 comments · Fixed by #565
Closed

Really strange scrolling issue when paging with firefox #562

tswfi opened this issue Nov 20, 2023 · 2 comments · Fixed by #565

Comments

@tswfi
Copy link
Contributor

tswfi commented Nov 20, 2023

Sometimes scrollTo(0,0) does not work in firefox. This is really strange and I cannot even reproduce this all the time, but enough to be really annoying :D

Steps to reproduce

  1. with "default demo products" change the pagesize to 3 (easier to get pager)
  2. go to "art" category (it should have three pages as the pagesize is small)
  3. scroll a bit to see the pager (important, also it seems that the viewport size also affects this a bit)
  4. change page
    =>
    window.scrollTo(0, 0);
    tries to scroll to top but it doesn't always happen (no errors, nothing).

It looks like firefox gets confused when the content changes and it doesn't always understand that it should scroll.. (also tried adding a small delay with setTimeout but that didn't seem to work either)

I can force it to work consistently by adding window.scrollTo(0,1) before the window.scrollTo(0,0) but that seems wrong.

This does not seem to ever happen with chrome.

As a side not the scrolling should probably happen to be at the top of <div id="products"> so that for mobile users the products are shown even if the header is tall.. In place of window.scrollTo(0,0) should be something like

document.getElementById('products').scrollIntoView(true); 

and as we have a fixed header give it some scroll margin

section#products {
  scroll-margin-top: 150px; /* really the height of header */
}

In my opinion the scroll-margin-top should be set so that in desktop view the text "There are 8 products" is still visible and in mobile view filters and sort order are still visible

tswfi added a commit to tswfi/hummingbird that referenced this issue Nov 20, 2023
Instead of doing scrollTo(0,0) use scrollIntoView to get the `section#products` start to be visible.

WIP, still requires a css rule
@Hlavtox
Copy link
Contributor

Hlavtox commented Nov 20, 2023

I am using this in my code and it seems to work properly.

$('html, body').animate({
  scrollTop: targetOffset
}, 400);

I set my theme to scroll to the top of product list with some margin when using pagination. And it doesn't sroll at all when using filters, it was annoying to scroll back down when you want to select more filters.

@tswfi
Copy link
Contributor Author

tswfi commented Nov 20, 2023

I can see how that would be bad when using filters.

So this requires even more design.. the event should know if it was paging operation or filter operation and then act accordingly.

tswfi added a commit to tswfi/hummingbird that referenced this issue Nov 28, 2023
nicosomb added a commit that referenced this issue Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants