[ Apps > Charts ] Fix "Charts" side nav item losing its highlighted state (#17332)

* ignore query params when comparing paths to highlight sidenav items

* address feedback

* lint

---------

Co-authored-by: Mo Mesgin <mmesgin@Mos-M2-MacBook-Pro.local>

Author: momesgin

shell/components/nav/Group.vue

@@ -82,7 +82,8 @@ export default {
           const validRoute = filterLocationValidParams(this.$router, overviewRoute || {});
           const route = this.$router.resolve(validRoute);
 
-          return this.$route.fullPath.split('#')[0] === route?.fullPath;
+          // Use .path instead of .fullPath to ignore query parameters and hashes when comparing routes
+          return this.$route.path === route?.path;
         }
       }
 
@@ -204,14 +205,14 @@ export default {
         } else if (item.route) {
           const navLevels = ['cluster', 'product', 'resource'];
           const matchesNavLevel = navLevels.filter((param) => !this.$route.params[param] || this.$route.params[param] !== item.route.params[param]).length === 0;
-          const withoutHash = this.$route.hash ? this.$route.fullPath.slice(0, this.$route.fullPath.indexOf(this.$route.hash)) : this.$route.fullPath;
-          const withoutQuery = withoutHash.split('?')[0];
           const validItemRoute = filterLocationValidParams(this.$router, item.route);
-          const itemFullPath = this.$router.resolve(validItemRoute).fullPath;
 
-          if (matchesNavLevel || itemFullPath === withoutQuery) {
+          // Use .path instead of .fullPath to ignore query parameters and hashes when comparing routes
+          const itemPath = this.$router.resolve(validItemRoute).path;
+
+          if (matchesNavLevel || itemPath === this.$route.path) {
             return true;
-          } else if (parentPath && itemFullPath === parentPath) {
+          } else if (parentPath && itemPath === parentPath) {
             return true;
           }
         }

shell/components/nav/Type.vue

@@ -68,8 +68,9 @@ export default {
     },
 
     isActive() {
-      const typeFullPath = this.$router.resolve(this.typeRoute)?.fullPath.toLowerCase();
-      const pageFullPath = this.$route.fullPath?.toLowerCase().split('#')[0]; // Ignore the shebang when comparing routes
+      // Use .path instead of .fullPath to ignore query parameters and hashes when comparing routes
+      const typePath = this.$router.resolve(this.typeRoute)?.path.toLowerCase();
+      const pagePath = this.$route.path?.toLowerCase();
       const routeMetaNav = this.$route.meta?.nav;
 
       // If the route explicitly declares the nav path that should be highlighted, then use that
@@ -80,14 +81,14 @@ export default {
           .replace(':cluster', cluster)
           .replace(':product', product);
 
-        if (navPath === typeFullPath) {
+        if (navPath === typePath) {
           return true;
         }
       }
 
       if ( !this.type.exact) {
-        const typeSplit = typeFullPath.split('/');
-        const pageSplit = pageFullPath.split('/');
+        const typeSplit = typePath.split('/');
+        const pageSplit = pagePath.split('/');
 
         for (let index = 0; index < typeSplit.length; ++index) {
           if ( index >= pageSplit.length || typeSplit[index] !== pageSplit[index] ) {
@@ -98,7 +99,7 @@ export default {
         return true;
       }
 
-      return typeFullPath === pageFullPath;
+      return typePath === pagePath;
     },
 
     typeRoute() {
@@ -131,7 +132,7 @@ export default {
   <router-link
     v-if="type.route"
     :key="type.name"
-    v-slot="{ href, navigate,isExactActive }"
+    v-slot="{ href, navigate, isExactActive }"
     custom
     :to="typeRoute"
   >

shell/components/nav/__tests__/Group.test.ts

@@ -0,0 +1,67 @@
+import { shallowMount } from '@vue/test-utils';
+import Group from '@shell/components/nav/Group.vue';
+
+describe('component: Group', () => {
+  it('isOverview ignores query parameters and hash strings when checking active state', () => {
+    const group = {
+      name:     'test',
+      children: [
+        {
+          route:    { name: 'overview-route' },
+          overview: true
+        }
+      ]
+    };
+
+    const wrapper = shallowMount(Group as any, {
+      props: {
+        group, canCollapse: true, idPrefix: ''
+      },
+      global: {
+        mocks: {
+          $route:  { path: '/test/route', fullPath: '/test/route?query=val#hash' },
+          $router: {
+            resolve:   jest.fn().mockReturnValue({ path: '/test/route', fullPath: '/test/route' }),
+            getRoutes: jest.fn().mockReturnValue([])
+          },
+          t: (key: string) => key
+        }
+      }
+    });
+
+    expect((wrapper.vm as any).isOverview).toBe(true);
+  });
+
+  it('hasActiveRoute ignores query parameters when checking item paths', () => {
+    const group = {
+      name:     'test',
+      children: [
+        { route: { name: 'child-route', params: {} } }
+      ]
+    };
+
+    const wrapper = shallowMount(Group as any, {
+      props: {
+        group, canCollapse: true, idPrefix: ''
+      },
+      global: {
+        mocks: {
+          $route: {
+            params:   {},
+            hash:     '#hash',
+            path:     '/child/route',
+            fullPath: '/child/route?query=val#hash',
+            matched:  []
+          },
+          $router: {
+            resolve:   jest.fn().mockReturnValue({ path: '/child/route', fullPath: '/child/route' }),
+            getRoutes: jest.fn().mockReturnValue([])
+          },
+          t: (key: string) => key
+        }
+      }
+    });
+
+    expect((wrapper.vm as any).hasActiveRoute()).toBe(true);
+  });
+});

shell/components/nav/__tests__/Type.test.ts

@@ -28,10 +28,10 @@ describe('component: Type', () => {
     };
     const routerMock = {
       resolve: jest.fn((route) => {
-        return { fullPath: route };
+        return { fullPath: route, path: route };
       })
     };
-    const routeMock = { fullPath: 'route' };
+    const routeMock = { fullPath: 'route', path: 'route' };
 
     describe('should pass props correctly', () => {
       it('should forward Type props to router-link', () => {
@@ -104,7 +104,7 @@ describe('component: Type', () => {
             directives: { cleanHtml: (identity) => identity },
 
             mocks: {
-              $store: storeMock, $router: routerMock, $route: { fullPath: 'bad' }
+              $store: storeMock, $router: routerMock, $route: { fullPath: 'bad', path: 'bad' }
             },
             stubs: { routerLink: createChildRenderingRouterLinkStub() },
           },
@@ -152,6 +152,23 @@ describe('component: Type', () => {
 
         expect(elementWithSelector.exists()).toBe(false);
       });
+      it('should use active and exact active classes when route matches but includes query and hash', () => {
+        const wrapper = shallowMount(Type as any, {
+          props: { type: defaultRouteTypeProp, highlightRoute: true },
+
+          global: {
+            directives: { cleanHtml: (identity) => identity },
+
+            mocks: {
+              $store: storeMock, $router: routerMock, $route: { fullPath: 'route?repo=test#myhash', path: 'route' }
+            },
+            stubs: { routerLink: createChildRenderingRouterLinkStub({ isExactActive: true }) },
+          },
+        });
+
+        expect(wrapper.find(`.${ activeClass }`).exists()).toBe(true);
+        expect(wrapper.find(`.${ exactActiveClass }`).exists()).toBe(true);
+      });
     });
 
     describe('should use classes if preconditions are met', () => {