fix: items with urls AND children should still navigate to their url

inventage · Dec 15, 2023 · 5b9bd9d · 5b9bd9d
1 parent 937f0b5
commit 5b9bd9d
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 15 deletions.
diff --git a/.changeset/ninety-shoes-punch.md b/.changeset/ninety-shoes-punch.md
@@ -0,0 +1,5 @@
+---
+'@inventage-web-components/portal-navigation': patch
+---
+
+Menu items with child items an `url` attribute navigate to their url instead of their default child item or the first child item.
diff --git a/packages/portal-navigation/data/data.json b/packages/portal-navigation/data/data.json
@@ -445,7 +445,8 @@
       },
       "items": [
         {
-          "defaultItem": "profile.preferences.userSettings",
+          "url": "/ebanking/preferences",
+          "application": "ebanking",
           "label": {
             "de": "Einstellungen",
             "en": "Preferences"
@@ -457,39 +458,39 @@
                 "de": "Benutzereinstellungen",
                 "en": "User Settings"
               },
-              "url": "/ebanking/update-user-preferences",
+              "url": "/ebanking/preferences/user",
               "application": "ebanking"
             },
             {
               "label": {
                 "de": "Sicherheit",
                 "en": "Security"
               },
-              "url": "/ebanking/update-security-preferences",
+              "url": "/ebanking/preferences/security",
               "application": "ebanking"
             },
             {
               "label": {
                 "de": "Darstellung",
                 "en": "Presentation"
               },
-              "url": "/ebanking/update-presentation-preferences",
+              "url": "/ebanking/preferences/presentation",
               "application": "ebanking"
             },
             {
               "label": {
                 "de": "Benachrichtigungen",
                 "en": "Notifications"
               },
-              "url": "/ebanking/update-notification-preferences",
+              "url": "/ebanking/preferences/notification",
               "application": "ebanking"
             },
             {
               "label": {
                 "de": "Mobiles Banking",
                 "en": "Mobile Banking"
               },
-              "url": "/ebanking/update-mobile-preferences",
+              "url": "/ebanking/preferences/mobile",
               "application": "ebanking"
             }
           ]

diff --git a/packages/portal-navigation/data/test-data.json b/packages/portal-navigation/data/test-data.json
@@ -119,7 +119,7 @@
         {
           "id": "parent3",
           "label": { "de": "Parent3_de", "en": "Parent3_en" },
-          "url": "/some/path/parent3",
+          "application": "app2",
           "items": [
             {
               "id": "item3.1",
@@ -152,7 +152,6 @@
         {
           "id": "parent5",
           "label": "Parent5",
-          "url": "/some/path/parent5",
           "expanded": true,
           "items": [
             {

diff --git a/packages/portal-navigation/src/Configuration.ts b/packages/portal-navigation/src/Configuration.ts
@@ -121,6 +121,14 @@ export interface SelectorFunction {
   (menu: CommonMenuItem): boolean;
 }
 
+export const isFirstLevelMenuItemOrMenuItem = (item: CommonMenuItem): item is FirstLevelMenuItem | MenuItem => {
+  return 'url' in item;
+};
+
+export const hasUrl = (item: CommonMenuItem): boolean => {
+  return isFirstLevelMenuItemOrMenuItem(item) && !!item.url;
+};
+
 /**
  * Wraps the json structured configuration of a portal navigation, does some basic sanitizing of the received data
  * (e.g. generating missing ids), and provides convenience functions to access menus and items with the data.

diff --git a/packages/portal-navigation/src/PortalNavigation.ts b/packages/portal-navigation/src/PortalNavigation.ts
@@ -6,7 +6,7 @@ import '@inventage-web-components/hamburger-menu/lib/src/hamburger-menu.js';
 
 import { IdPath } from './IdPath.js';
 import { styles } from './styles-css.js';
-import { CommonMenuItem, Configuration, FirstLevelMenuItem, MenuItem, MenuLabel } from './Configuration.js';
+import { CommonMenuItem, Configuration, FirstLevelMenuItem, hasUrl, isFirstLevelMenuItemOrMenuItem, MenuItem, MenuLabel } from './Configuration.js';
 
 /**
  * A listing of key menu ids that are handled specifically by the portal navigation component.
@@ -972,7 +972,7 @@ export class PortalNavigation extends LitElement {
       return;
     }
 
-    const hasItems = !!item.items && item.items.length > 0;
+    const hasItems = !item.url && !!item.items && item.items.length > 0;
     const internalRouting = this.__isInternalRouting(item);
     const { expanded = false } = item as Record<string, boolean>;
 
@@ -1020,7 +1020,7 @@ export class PortalNavigation extends LitElement {
    */
   private __isInternalRouting(item?: MenuItem): boolean {
     let refItem: MenuItem | undefined = item;
-    if (item && item.items && item.items.length > 0) {
+    if (item && !item.url && item.items && item.items.length > 0) {
       refItem = <MenuItem>this.__getDefaultItemOf(item);
     }
 
@@ -1064,8 +1064,8 @@ export class PortalNavigation extends LitElement {
     let dispatchEvent = true;
     let closeHamburgerExpanded = true;
     if (hasItems) {
-      refItem = this.__getDefaultItemOf(selectedItem!);
-      dispatchEvent = !!refItem && !this.isMobileBreakpoint && refItem.destination !== 'extern';
+      refItem = hasUrl(selectedItem) ? selectedItem : this.__getDefaultItemOf(selectedItem!);
+      dispatchEvent = !this.isMobileBreakpoint && !!refItem && isFirstLevelMenuItemOrMenuItem(refItem) && refItem.destination !== 'extern';
 
       // Expanded hamburger should be closed only when the clicked item does not have an internal default item
       // This ensures that accordions in mobile breakpoint can be expanded. The routeTo event will still be thrown

diff --git a/packages/portal-navigation/test/PortalNavigation.test.ts b/packages/portal-navigation/test/PortalNavigation.test.ts
@@ -695,14 +695,14 @@ describe('<portal-navigation>', () => {
       await childrenRendered(el);
 
       const clickElement = <HTMLAnchorElement>el.shadowRoot!.querySelector('[part~="item-parent5"]');
-      const clicked = oneEvent(clickElement, 'click');
+      const clicked = oneEvent(document, 'click');
       setTimeout(() => clickElement.click());
       await clicked;
 
       expect(eventSpy.callCount).to.equal(0);
       expect(el.getActivePath().getMenuId()).to.equal('logout');
-      expect(el.getActivePath().getId(2)).to.be.undefined;
       expect(el.getActivePath().getFirstLevelItemId()).to.equal('parent5');
+      expect(el.getActivePath().getId(2)).to.be.undefined;
     });
 
     it('dispatches the "routeTo" event', async () => {
@@ -732,6 +732,17 @@ describe('<portal-navigation>', () => {
       expect(window.location.pathname, 'window.location.pathname').to.equal('/');
       expect(detail.url, 'detail.url').to.equal('/some/path/parent1');
     });
+
+    it('dispatches the "routeTo" event when item has children but also has a URL set', async () => {
+      const el: PortalNavigation = await fixture(html` <portal-navigation src="${TEST_DATA_JSON_PATH}" currentapplication="app1"></portal-navigation>`);
+      await childrenRendered(el);
+
+      setTimeout(() => (<HTMLAnchorElement>el.shadowRoot!.querySelector('[part~="item-parent8"]')).click());
+      const { detail } = await oneEvent(el, 'portal-navigation.routeTo');
+
+      expect(window.location.pathname, 'window.location.pathname').to.equal('/');
+      expect(detail.url, 'detail.url').to.equal('/some/path/parent8?foo=bar');
+    });
   });
 
   describe('Translations', () => {