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

Scrolling is little irritating #970

Closed
sync-by-unito bot opened this issue May 9, 2022 · 11 comments
Closed

Scrolling is little irritating #970

sync-by-unito bot opened this issue May 9, 2022 · 11 comments
Assignees
Labels

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented May 9, 2022

For example:

  1. Go to Women > Tops
  2. Choose filter for black products, apply
  3. Now you must click somewhere to scroll the page

Acceptance criteria

  1. After clicking “apply filters” or “clear all” buttons, a user should be able to scroll through the content
  2. After clicking “apply filters” or “clear all” buttons, the page should be scrolled to top of the product list

┆Attachments: Screenshot 2022-05-09 at 14.31.06.png

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 10, 2022

➤ Bartosz Herba commented:

Marcin Kwiatkowski Makes sense, we should also make sure that after applying filters we scroll to the category navbar not the very top of the page (currently we jump to the very top with cms content which is not really what user want to see at that time)

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 19, 2022

➤ Artur Tagisow commented:

fix: prevent SfSidebar disableBodyScroll triggering on desktop

As part of vuestorefront/storefront-ui#619 ( https://github.com/vuestorefront/storefront-ui/pull/619|smart-link )
body-scroll-lock was added to SfSidebar.
If you look closely at
https://github.com/vuestorefront/storefront-ui/blob/d5b646a6344e0ab4f5dad214458662af7c8bbd57/packages/vue/src/components/organisms/SfSidebar/SfSidebar.vue#L146 ( https://github.com/vuestorefront/storefront-ui/blob/d5b646a6344e0ab4f5dad214458662af7c8bbd57/packages/vue/src/components/organisms/SfSidebar/SfSidebar.vue#L146|smart-link )
you'll see that disableBodyScroll is called in a watcher on the "visible" prop.
hint: disableBodyScroll() just sets "overflow: hidden" on the HTML
document body, which makes the body unscrollable.
In this case that I'm fixing, it goes like:

  1. The value passed to CategoryFilters's isVisible comes from
    useUiState's isFilterSidebarOpen. This value is false by default.
  2. Clicking the "clear" or "apply" button in the filters emits the
    "close" event from CategoryFilters - if you look at category.vue that
    just calls "toggleFilterSidebar". This will set isVisible to TRUE
  3. Changing isVisible to true triggers SfSidebar's watcher, even though the SfSidebar
    it is not currently visible (it should be visible only on mobile)
  4. This in turn runs disableBodyLock() and prevents scrolling

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 19, 2022

➤ Artur Tagisow commented:

To QA: I checked a bit but I’m worried that this could introduce some issues with the filter sidebar, particularly on mobile. Please check if there isn’t any unwanted behavior - it could be some issues like eg. add filters in sidebar, hide sidebar, make sidebar appear again → filters don’t persist - or something similar. If they don’t happen, great

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 20, 2022

➤ Bartosz Herba commented:

Artur Tagisow Could you create an issue ticket for the SFUI?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 20, 2022

➤ Artur Tagisow commented:

We’re SfSidebar in a non-standard way though - there is a custom our own sidebar for the filters, but SfSidebar for the mobile view. I think Storefrount UI intends SfSidebar to be used uniformly so our case is unsupported Bartosz Herba

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 20, 2022

➤ Artur Tagisow commented:

Not sure if I should assign Tomasz Korczak or Gabriel Golla

@sync-by-unito
Copy link
Author

sync-by-unito bot commented May 20, 2022

➤ Gabriel Golla commented:

Artur Tagisow assign everything to me 😉

@Frodigo Frodigo closed this as completed Jul 5, 2022
@sync-by-unito
Copy link
Author

sync-by-unito bot commented Jul 6, 2022

➤ Gabriel Golla commented:

To be honest I don’t see any changes… Issue reported by Tomek is still occurring.

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Jul 6, 2022

➤ Gabriel Golla commented:

OK, scrolling after applying/clearing filters works good and page is scrolled up to the top. Confirmed using following browsers:

  1. Chrome desktop (102) and mobile (101)
  2. Safari desktop (15.4) and mobile (15.4)
  3. Edge desktop (101) and mobile (100)
  4. Firefox desktop (100) and mobile (99)

Milosh Belter would take a look also on your iPhone? Unfortunately we still don’t have full access to Lambda Test…

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Jul 6, 2022

➤ Milosh Belter commented:

Gabriel Golla just checked it on iPhone 12 - it’s working fine for me - no issues (both Safari and Chrome)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants