Layered Navigation - Desktop Optimizations #3137

justinconabree · 2021-04-21T18:03:41Z

Description

Display the layered navigation contents directly on the category and search pages when on desktop (equal to or above 1024px).
Changed the markup/layout of the category and search pages to support the left column on desktop.
Moved and styled the Sort button on desktop. It now also includes the current sorting within the button text.
Added X Results messaging above the results on category pages (both mobile and desktop)
Added Show More / Show Less button within the filter list when there are more than 5 items (both mobile and desktop). You can also pass in a new prop "showItems" to the FilterList change this threshold
Open the first 3 filter accordions on desktop. You can also pass in a new prop "filtersOpen" to the FilterSidebar change this threshold
Also open any filters that have selected items (both mobile and desktop)
Scroll to the top of filters (-150px for menu threshold) when changing the filters on desktop

Related Issue

Closes #3115

Acceptance

Verification Stakeholders

Specification

Verification Steps

Go to the Bottoms category page on a desktop browser
Verify the layered nav is visible on the left
Verify the sort button is on the right and displays current sort order within button
Verify the first 3 filters are open and the rest are closed
Verify the Show More/Less works correctly within the Color attribute list
Verify selecting a filter in one of the closed attributes (ex Size) then forces the accordion open (but can still be manually closed)
Verify selecting a filter lower down on the page scrolls the viewport back to the top of the filter list
Verify that the mobile filters reflect what was selected on desktop and function as they did before these modifications
Search for "pants"
Verify the layout is functioning as it did in the category page

Screenshots / Screen Captures (if appropriate)

Checklist

[ ] I have added tests to cover my changes, if necessary.
[x] I have updated existing tests to cover my changes, if necessary.
[x] I have added translations for new strings, if necessary.
[x] I have updated the documentation accordingly, if necessary.

…rch layout and styles for sidebar, keep filter accordion open if selected or in first 3 (magento#3115)

…unt (magento#3115)

…nd add icon, implement show more/less in filter list, scroll to top on apply filter, add total number of results in toolbar, add filter header

… move productSort styles to productSort component (magento#3115)

PWAStudioBot · 2021-04-21T18:15:11Z

	Messages
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 938c0eb

fooman · 2021-05-02T23:04:17Z

Not sure if this is a graphql issue or expected. On https://pr-3137.pwa-venia.com/shop-the-look.html?page=1 you start out with 10 Results

Clicking the first price filter cuts down the results to 2

Enabling the second filter I would anticipate seeing all products again. However it is still stuck at 2

fooman · 2021-05-06T03:15:55Z

Just to confirm selecting multiple price bands seems to be problematic prior to this PR - for example https://pr-3146.pwa-venia.com/shop-the-look.html?page=1&price%5Bfilter%5D=100-200%2C100_200&price%5Bfilter%5D=0-100%2C0_100

sirugh · 2021-05-06T14:07:24Z

Just to confirm selecting multiple price bands seems to be problematic prior to this PR - for example https://pr-3146.pwa-venia.com/shop-the-look.html?page=1&price%5Bfilter%5D=100-200%2C100_200&price%5Bfilter%5D=0-100%2C0_100

Thanks @fooman, opened #3161

sirugh

This looks great! Thank you so much for your patience while we worked to get the accessibility PR merged. I've left some feedback for you! A large number of the comments are just me identifying a prop rename, but there are a few things in here I'd like to see addressed if possible.

Please let me know if you have time to address this review or if you need someone on the internal team to take it over from here.

packages/venia-ui/lib/RootComponents/Category/category.css

packages/peregrine/lib/talons/FilterModal/useFilterModal.js

sirugh · 2021-05-11T17:25:24Z

packages/venia-ui/lib/components/FilterModal/filterModal.css

@@ -1,5 +1,5 @@
 .root {
-    --stroke: var(--venia-global-color-border);
+    --stroke: rgb(var(--venia-global-color-border));


Just an FYI, this might conflict with a fix incoming in #3171.

cc @tjwiebell

packages/venia-ui/lib/components/FilterModal/FilterList/filterList.js

packages/venia-ui/lib/components/FilterModal/CurrentFilters/currentFilter.js

packages/venia-ui/lib/components/FilterModal/filterBlock.js

packages/venia-ui/lib/components/FilterSidebar/filterSidebar.js

sirugh · 2021-05-11T20:57:27Z

packages/venia-ui/lib/components/FilterSidebar/filterSidebar.js

+ */
+const FilterSidebar = props => {
+    const { filters, filtersOpen } = props;
+    const talonProps = useFilterModal({ filters });


We don't have this really documented anywhere, and we've recently run into this with the WishlistButton work, but the relationship between talons and components is 1:1 to avoid breaking changes within the talon affecting a related component that isn't quite identical to the original component.

So instead of reusing useFilterModal we should create a new talon called useFilterSidebar that is a duplicate of the other. You'll then want to put the sidebar-specific logic such as the handleApplyFilter callback creation within this new talon.

I appreciate your attempt at re-use here, and one day we might have a better example of generalized logic, but for now just create the unique talon please :)

Edit:

As a followup, I was curious if you attempted to try to reuse/rename FilterModal and just have a mobile view vs desktop view. I assume the differences warranted the new component, but still curious :)

@sirugh It hurts to duplicate that much code, but I'll do it for you ;). The state should hopefully cross over since it's essentially based off of the URL, but I'll test to be sure.

The work is slightly based off of one of our projects and their approach was to use the same component. However there were too many "if" conditions for it to be maintainable long term, so we went for the approach of having a separate component.

It hurts to duplicate that much code

I know :/ but the alternative is bug surface (for now). Eventually we will have a pattern for duplicated logic for talons, but we don't want to solve that here :)

packages/venia-ui/lib/components/SearchPage/searchPage.js

sirugh

Ok, good to go again.

sirugh · 2021-05-14T23:29:37Z

@JustinConabreeAbsolunet do you have time to add test coverage to the new functionality? Looks like the CI is failing due to a ~1% drop in coverage. If you don't have time to do it we can get an internal developer to do it, so no worries - just let me know what works for you. Thanks!

justinconabree · 2021-05-17T12:36:07Z

@sirugh Sure! I'll get that done today.

…alons

justinconabree · 2021-05-17T16:38:27Z

@sirugh I copied over the useFilterModal test to the useFilterSidebar since the functionality is the same. And I added two new tests for the useFilterList and useFilterBlock. I think it's passing now. Let me know if you need more tests

sirugh · 2021-05-17T17:06:57Z

Thanks Justin, we're almost there!

If you click the details link you can see what is causing the decrease. I think at a minimum we require 0% loss of coverage to files with tests, but I can't find our config...

justinconabree · 2021-05-17T17:09:28Z

@sirugh yeah I saw right after posting the message. There was just enough delay for the coveralls test.
I'll implement some tests on the components then to get it to 0%!

…t/desktop-layered-nav-optimizations

…com:JustinConabreeAbsolunet/pwa-studio into absolunet/desktop-layered-nav-optimizations

…t/desktop-layered-nav-optimizations

anthoula · 2021-05-19T19:11:24Z

@JustinConabreeAbsolunet Thanks Justin, I have this observation in Lighthouse. There is a new warning introduced Avoid an excessive DOM size. Is it possible to do some exploration to see if we can avoid this new warning?

Note: Refactored functional tests to accommodate mobile specific selectors, due to duplication in the DOM.

justinconabree · 2021-05-19T20:45:00Z

Hi @anthoula ,

Unfortunately this is a byproduct of duplicating the dom of the layered nav for mobile and desktop. This is the same approach that was done with the mega-menu, and there's no good way to get around it while supporting SSR.

I'm open for suggestions, but this was the approach discussed with the PWA Studio team before we started the implementation, since there's nothing in place yet to handle different markup between desktop and mobile.

anthoula · 2021-05-19T21:18:44Z

@JustinConabreeAbsolunet Second observation: Search for Tops, then click on the Price label for 100-200. Notice that on the first click, it does not select the filter, but on the second click of the label, it selects.

Related Observation: Search for Tops, then click on the Sort by ... button. Notice that on first click, the dropdown does not appear, but on the second click, the dropdown appears.

justinconabree · 2021-05-19T21:56:15Z

@anthoula This seems to be caused by the search bar still being open and trapping the click elsewhere on the page. If you click elsewhere and then on the filter it works correctly.

This seems to have been the case prior to this PR as well
https://pr-3146.pwa-venia.com/
If you search for tops and try to click on the Filters button, nothing happens.

In my opinion it should be a separate issue, but I can work on it in this PR if you need.

anthoula · 2021-05-20T21:53:36Z

Thanks for the contribution @JustinConabreeAbsolunet ! We will take a closer look at these items in a separate ticket.

…t/desktop-layered-nav-optimizations

anthoula · 2021-05-26T22:09:34Z

QA approved

justinconabree added 5 commits April 19, 2021 13:17

Adding FilterSidebar component for desktop, refactor category and sea…

3c50dab

…rch layout and styles for sidebar, keep filter accordion open if selected or in first 3 (magento#3115)

Adding prop to FilterSidebar to allow easy changing of open filter co…

2a8e9eb

…unt (magento#3115)

Fix current filters, cleanup search page, change sort button styles a…

e6cc96f

…nd add icon, implement show more/less in filter list, scroll to top on apply filter, add total number of results in toolbar, add filter header

Updating snapshots to pass, fix useCategoryContent to handle no data,…

57579ea

… move productSort styles to productSort component (magento#3115)

Fixing sidebar positioning (magento#3115)

8f20e76

magento-engcom-team added Partner: Absolunet Inc partners-contribution labels Apr 21, 2021

m2-community-project bot added the Progress: ready for review label Apr 21, 2021

PWAStudioBot added pkg:extensions pkg:peregrine pkg:venia-ui labels Apr 21, 2021

Fixing lint and prettier issues

66d3ec2

sirugh mentioned this pull request May 6, 2021

#1222 Filters (Layered Navigation) - Add keyboard focus and adjust the way tab order should work #3034

Merged

PWAStudioBot added pkg:pagebuilder pkg:pwa-buildpack pkg:venia-concept labels May 10, 2021

justinconabree force-pushed the absolunet/desktop-layered-nav-optimizations branch from ec880c4 to 66d3ec2 Compare May 11, 2021 14:19

justinconabree added 2 commits May 11, 2021 11:18

Merge branch 'develop' into absolunet/desktop-layered-nav-optimizations

2b85f41

Fixing merge issue with chevron color, show more label and prettier

d0b11fa

sirugh added the version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump. label May 11, 2021

sirugh requested changes May 11, 2021

View reviewed changes

m2-community-project bot assigned sirugh May 11, 2021

m2-community-project bot added Progress: changes requested and removed Progress: ready for review labels May 11, 2021

sirugh previously approved these changes May 12, 2021

View reviewed changes

Adding tests for useFilterSidebar, useFilterBlock and useFilterList t…

2285567

…alons

justinconabree dismissed sirugh’s stale review via 2285567 May 17, 2021 16:05

Fixing useFilterSidebar test, running prettier

a7bc565

justinconabree and others added 4 commits May 17, 2021 15:45

Adding test for filterModel and filterSidebar

d48d5a7

Merge branch 'develop' of github.com:magento/pwa-studio into absolune…

f48b9e7

…t/desktop-layered-nav-optimizations

Adding test coverage for filterItem

58482cc

Merge branch 'absolunet/desktop-layered-nav-optimizations' of github.…

7c27800

…com:JustinConabreeAbsolunet/pwa-studio into absolunet/desktop-layered-nav-optimizations

sirugh approved these changes May 18, 2021

View reviewed changes

anthoula and others added 2 commits May 18, 2021 12:57

Merge branch 'develop' of github.com:magento/pwa-studio into absolune…

4f49967

…t/desktop-layered-nav-optimizations

Merge branch 'develop' into absolunet/desktop-layered-nav-optimizations

ae9e52d

anthoula added 3 commits May 26, 2021 13:37

Merge branch 'develop' of github.com:magento/pwa-studio into absolune…

637be1c

…t/desktop-layered-nav-optimizations

Merge branch 'develop' of github.com:magento/pwa-studio into absolune…

62b4c01

…t/desktop-layered-nav-optimizations

Merge branch 'develop' of github.com:magento/pwa-studio into absolune…

938c0eb

…t/desktop-layered-nav-optimizations

anthoula merged commit f613b61 into magento:develop May 26, 2021

m2-community-project bot added Progress: done and removed Progress: approved labels May 26, 2021

justinconabree mentioned this pull request Jul 20, 2021

Regression Fix - Content shifted under layered nav on PLP #3285

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layered Navigation - Desktop Optimizations #3137

Layered Navigation - Desktop Optimizations #3137

justinconabree commented Apr 21, 2021

PWAStudioBot commented Apr 21, 2021 •

edited

Loading

fooman commented May 2, 2021 •

edited

Loading

fooman commented May 6, 2021

sirugh commented May 6, 2021

sirugh left a comment

sirugh May 11, 2021

sirugh May 11, 2021 •

edited

Loading

justinconabree May 12, 2021

sirugh May 12, 2021

sirugh left a comment

sirugh commented May 14, 2021

justinconabree commented May 17, 2021

justinconabree commented May 17, 2021

sirugh commented May 17, 2021

justinconabree commented May 17, 2021

anthoula commented May 19, 2021 •

edited

Loading

justinconabree commented May 19, 2021

anthoula commented May 19, 2021 •

edited

Loading

justinconabree commented May 19, 2021

anthoula commented May 20, 2021

anthoula commented May 26, 2021

Layered Navigation - Desktop Optimizations #3137

Layered Navigation - Desktop Optimizations #3137

Conversation

justinconabree commented Apr 21, 2021

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Apr 21, 2021 • edited Loading

fooman commented May 2, 2021 • edited Loading

fooman commented May 6, 2021

sirugh commented May 6, 2021

sirugh left a comment

Choose a reason for hiding this comment

sirugh May 11, 2021

Choose a reason for hiding this comment

sirugh May 11, 2021 • edited Loading

Choose a reason for hiding this comment

justinconabree May 12, 2021

Choose a reason for hiding this comment

sirugh May 12, 2021

Choose a reason for hiding this comment

sirugh left a comment

Choose a reason for hiding this comment

sirugh commented May 14, 2021

justinconabree commented May 17, 2021

justinconabree commented May 17, 2021

sirugh commented May 17, 2021

justinconabree commented May 17, 2021

anthoula commented May 19, 2021 • edited Loading

justinconabree commented May 19, 2021

anthoula commented May 19, 2021 • edited Loading

justinconabree commented May 19, 2021

anthoula commented May 20, 2021

anthoula commented May 26, 2021

PWAStudioBot commented Apr 21, 2021 •

edited

Loading

fooman commented May 2, 2021 •

edited

Loading

sirugh May 11, 2021 •

edited

Loading

anthoula commented May 19, 2021 •

edited

Loading

anthoula commented May 19, 2021 •

edited

Loading