feat(theme): support navigating to nested categories in mobile side menu #709

sethidden · 2022-03-09T08:43:47Z

Description

Add the ability to enter subcategories from the side menu on mobile
Before this PR you could only access top level categories like Men, Women, Gear, but now you can enter eg. Men>Tops>Jackets

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Screen.Recording.2022-03-10.at.12.55.52.mov

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

sethidden · 2022-03-10T09:08:20Z

Wait for refactor(composables)!: align refactor perf improvements branch to current develop #713 to be merged then fix conflicts

sethidden · 2022-03-10T09:34:54Z

Move MobileMenuSidebar to the catalog/category module and change its name so its not misleading
Remove useSidebar category functions and put it in separate file that's close to MobileCATEGORYMenuSidebar
Maybe break down useSidebar and move to dumb helpers module

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue

sethidden · 2022-03-10T12:02:40Z

.../modules/catalog/category/components/sidebar/MobileCategorySidebar/MobileCategorySidebar.vue

@@ -0,0 +1,129 @@
+<template>


Loads of diffs because I just moved the file, but there were plenty of changes to the template and script

sethidden · 2022-03-10T12:03:19Z

.../modules/catalog/category/components/sidebar/MobileCategorySidebar/MobileCategorySidebar.vue

+      router.push(path);
+    };
+
+    const activeCategory: CategoryTreeInterface = findDeep(categoryTree.value, (value, key) => key === 'slug' && value === route.value.fullPath.replace('/default/c', ''))?.parent;


Note the ugly hack with the /default/c replace, but that's the closest I could find to a "unique id" to detect the current category.

packages/theme/tests/e2e/tsconfig.json

sethidden · 2022-03-10T12:16:50Z

Hmm the Jira description says The MobileMenuSidebar component is lazy-loaded, not sure if that's done

sethidden · 2022-03-10T12:19:16Z

The useCategory and categoryGetters are removed from MobileMenuSidebar componet this is also not done yet

packages/theme/components/BottomNavigation.vue

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue

sethidden · 2022-03-14T08:53:34Z

packages/theme/modules/catalog/category/components/sidebar/MobileCategorySidebar/logic.ts

+import { CategoryTreeInterface } from '~/modules/catalog/category/types';
+
+export const useMobileCategoryTree = (initialHistory: CategoryTreeInterface[] = []) => {
+  const history = ref<CategoryTreeInterface[]>(initialHistory);


There's something really wrong with TypeScript here as it can't understand the <T = CategoryTreeInterface> syntax, so there's lots of duplication of types

packages/theme/modules/catalog/category/components/sidebar/command/categoryList.gql.ts

sethidden · 2022-03-14T08:59:07Z

need to resolve conflicts, sec

sethidden · 2022-03-14T09:12:41Z

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue

+    const activeCategoryAncestors = ref(null);
+    const isLoading = ref(true);

    onMounted(async () => {


Shouldn't this be useAsync?

Probably out of scope for this PR, but still

The onMounted is good here, because we won't cache the category tree in Redis/cdn so they are loaded on the client-side.

Before this commit there were 2 issues: - Types from Cypress and Jest fighting for the `expect()` keyword in .spec.ts files but this happened only when running tests with Jest (ts-jest) - TypeScript didn't know where to find the @types/jest (when using tsserver), so I changed the typeRoots to point to the parent pkg This is only a half-solution as I think if I used some composite configs it'd be more manageable fix(theme): squash this fix(theme): squash this

refactor(theme): refactor find active category function to return whole object squash this feat(theme): pinia working wip feat(theme): reorganize folders for helpers feat(theme): reduce duplication in graphql query feat(theme): adjust desktop and mobile categories fix(theme): fix invalid import paths

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue

Frodigo · 2022-03-15T08:22:19Z

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue

+    const activeCategoryAncestors = ref(null);
+    const isLoading = ref(true);

    onMounted(async () => {


The onMounted is good here, because we won't cache the category tree in Redis/cdn so they are loaded on the client-side.

packages/theme/tests/e2e/tsconfig.json

…enu (#709) * fix(theme): allow spec.ts files to be used Before this commit there were 2 issues: - Types from Cypress and Jest fighting for the `expect()` keyword in .spec.ts files but this happened only when running tests with Jest (ts-jest) - TypeScript didn't know where to find the @types/jest (when using tsserver), so I changed the typeRoots to point to the parent pkg This is only a half-solution as I think if I used some composite configs it'd be more manageable fix(theme): squash this fix(theme): squash this * feat(theme): add nested categories in mobile category menu refactor(theme): refactor find active category function to return whole object squash this feat(theme): pinia working wip feat(theme): reorganize folders for helpers feat(theme): reduce duplication in graphql query feat(theme): adjust desktop and mobile categories fix(theme): fix invalid import paths

github-actions bot assigned sethidden Mar 9, 2022

sethidden marked this pull request as ready for review March 10, 2022 09:08

sethidden force-pushed the bugfix/M2-220-categories-in-mobile-menu branch from 325114a to 86367f4 Compare March 10, 2022 11:49

sethidden requested review from Frodigo and bartoszherba March 10, 2022 12:01

sethidden commented Mar 10, 2022

View reviewed changes

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue Show resolved Hide resolved

sethidden commented Mar 10, 2022

View reviewed changes

packages/theme/tests/e2e/tsconfig.json Show resolved Hide resolved

sethidden force-pushed the bugfix/M2-220-categories-in-mobile-menu branch 2 times, most recently from ee65681 to 7296abf Compare March 10, 2022 12:08

sethidden changed the title ~~Bugfix/m2 220 categories in mobile menu~~ feat(theme): support navigating to nested categories in mobile side menu Mar 10, 2022

sethidden removed request for Frodigo and bartoszherba March 10, 2022 12:28

sethidden marked this pull request as draft March 10, 2022 12:29

sethidden force-pushed the bugfix/M2-220-categories-in-mobile-menu branch 2 times, most recently from a3d1e41 to 9dbab82 Compare March 10, 2022 14:28

sethidden commented Mar 14, 2022

View reviewed changes

packages/theme/components/BottomNavigation.vue Show resolved Hide resolved

sethidden commented Mar 14, 2022

View reviewed changes

packages/theme/modules/catalog/category/components/sidebar/CategorySidebar.vue Show resolved Hide resolved

sethidden commented Mar 14, 2022

View reviewed changes

packages/theme/modules/catalog/category/components/sidebar/command/categoryList.gql.ts Show resolved Hide resolved

sethidden marked this pull request as ready for review March 14, 2022 08:58

sethidden commented Mar 14, 2022

View reviewed changes

sethidden force-pushed the bugfix/M2-220-categories-in-mobile-menu branch from 02cd541 to 9143eea Compare March 14, 2022 11:33

sethidden force-pushed the bugfix/M2-220-categories-in-mobile-menu branch from 9143eea to 060fe77 Compare March 14, 2022 11:39

sethidden requested review from Frodigo and bartoszherba March 14, 2022 11:44

Frodigo approved these changes Mar 15, 2022

View reviewed changes

Frodigo merged commit 1b11e9c into develop Mar 15, 2022

Frodigo deleted the bugfix/M2-220-categories-in-mobile-menu branch March 15, 2022 08:39

Frodigo added this to the 1.0.0-rc.7 milestone Mar 15, 2022

Frodigo mentioned this pull request May 10, 2022

chore: release 1.0.0-rc.7 #949

Merged

Frodigo mentioned this pull request May 25, 2022

chore: release 1.0.0-rc.7 vuestorefront/template-magento#23

Merged

feat(theme): support navigating to nested categories in mobile side menu #709

feat(theme): support navigating to nested categories in mobile side menu #709

Uh oh!

Conversation

sethidden commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

sethidden commented Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sethidden commented Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sethidden Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

sethidden Mar 10, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sethidden commented Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sethidden commented Mar 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sethidden Mar 14, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sethidden commented Mar 14, 2022

Uh oh!

sethidden Mar 14, 2022

Choose a reason for hiding this comment

Uh oh!

sethidden Mar 14, 2022

Choose a reason for hiding this comment

Uh oh!

Frodigo Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Frodigo Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sethidden commented Mar 9, 2022 •

edited

Loading

sethidden commented Mar 10, 2022 •

edited

Loading

sethidden commented Mar 10, 2022 •

edited

Loading

sethidden commented Mar 10, 2022 •

edited

Loading

sethidden commented Mar 10, 2022 •

edited

Loading