Skip to content

Commit

Permalink
fix(#1569): side menu not showing current page correctly when there i…
Browse files Browse the repository at this point in the history 
…s sub item
  • Loading branch information
@vanessatran-ddi
vanessatran-ddi committed May 27, 2024
1 parent a1d3ea0 commit 6300043
Show file tree
Hide file tree
Showing 11 changed files with 320 additions and 234 deletions.
115 changes: 59 additions & 56 deletions libs/web-components/src/common/urls.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ it("should match urls", async () => {
desc: "empty test url",
windowUrl: new URL("http://localhost/foo"),
testUrl: "",
weight: 0,
weight: -1,
},
{
desc: "root path only match",
windowUrl: new URL("http://localhost"),
testUrl: "/",
weight: 0,
weight: 1,
},
{
desc: "path match",
Expand Down Expand Up @@ -121,25 +121,37 @@ it("should match urls", async () => {
];

for (const spec of specs) {
expect(isUrlMatch(spec.windowUrl, spec.testUrl)).toEqual(spec.weight);
try {
expect(isUrlMatch(spec.windowUrl, spec.testUrl)).toEqual(spec.weight);
} catch (error) {
throw new Error(spec.desc);
}

}
});

describe("should getMatchedLink", () => {
interface MenuTest {
desc: string;
windowUrl: URL;
activeMenuHref: string|undefined;
}

it("should fix bug/1368 getMatchedLink", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const links: any[] = [
{
getAttribute: (attr: string) => attr === 'href' ? "#/" : null,
getAttribute: (attr: string) => attr === 'href' ? "/" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "#/get-started" : null,
getAttribute: (attr: string) => attr === 'href' ? "/get-started" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "#/tabs" : null,
getAttribute: (attr: string) => attr === 'href' ? "/accordion" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "/patterns" : null,
},
// Make sure external link won't be able to highlighted in any case, even it matched
{
getAttribute: (attr: string) => {
if (attr === 'href') return "https://google.com/choose";
Expand All @@ -149,54 +161,45 @@ describe("should getMatchedLink", () => {
}
];

const specs: MenuTest[] = [
{
desc: "return home menu / if we navigate to /",
windowUrl: new URL("http://localhost/"),
activeMenuHref: "/"
},
{
desc: "return Get started menu if we navigate to /get-started",
windowUrl: new URL("http://localhost/get-started"),
activeMenuHref: "/get-started"
},
{
desc: "return Get started menu if we navigate to /get-started/developers",
windowUrl: new URL("http://localhost/get-started/developers"),
activeMenuHref: "/get-started"
},
{
desc: "return Accordion if we navigate to /accordion#tab-0",
windowUrl: new URL("http://localhost/accordion#tab-0"),
activeMenuHref: "/accordion"
},
{
desc: "return patterns menu if we navigate to /patterns#tab-1",
windowUrl: new URL("http://localhost/patterns#tab-1"),
activeMenuHref: "/patterns"
},
{
desc: "return no menu if we navigate to /profile",
windowUrl: new URL("http://localhost/profile"),
activeMenuHref: undefined
}
];

it("should return null if we navigate to a home root / (React app)", () => {
const windowUrl = "/";
const result = getMatchedLink(links, windowUrl);
expect(result).toBeNull();
})

it("should return Home menu if we navigate to a root #/ (Angular app)", () => {
const windowUrl = "/ui-components/#/";
const result = getMatchedLink(links, windowUrl);
expect(result?.getAttribute("href")).toEqual("#/");
});

it("should return get-started if we navigate to /get-started", () => {
const windowUrl = "/ui-components/#/get-started";
const result = getMatchedLink(links, windowUrl);
expect(result?.getAttribute("href")).toEqual("#/get-started");
});

it("should return get-started if we navigate to /get-started/developers", () => {
const windowUrl = "/ui-components/#/get-started/developers";
const result = getMatchedLink(links, windowUrl)
expect(result?.getAttribute("href")).toEqual("#/get-started");
});

it("should return tabs if we navigate to /tabs#tab-0", () => {
const windowUrl = "/ui-components/#/tabs#tab-0";
const result = getMatchedLink(links, windowUrl);
expect(result?.getAttribute("href")).toEqual("#/tabs");
});

it("should return null if we navigate to /accordion", () => {
const windowUrl = "/ui-components/#/accordion";
const result = getMatchedLink(links, windowUrl);
console.log(result?.getAttribute("href"));
expect(result).toBeNull();
});

it("should return patterns menu if we navigate to /patterns", () => {
const windowUrl = "/patterns#tab-0";
const result = getMatchedLink(links, windowUrl);
expect(result?.getAttribute("href")).toEqual("/patterns");
});

it("should return patterns menu if we navigate to /patterns/complex-form", () => {
const windowUrl = "/patterns/complex-form";
const result = getMatchedLink(links, windowUrl);
expect(result?.getAttribute("href")).toEqual("/patterns");
});

for (const spec of specs) {
const matchedLink = getMatchedLink(links, spec.windowUrl);
try {
expect(matchedLink?.getAttribute("href")).toEqual(spec.activeMenuHref);
} catch (error) {
throw new Error(spec.desc);
}
}
})
61 changes: 8 additions & 53 deletions libs/web-components/src/common/urls.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@ export function isUrlMatch(windowUrl: URL | Location, testUrl: string): number {
return 1;
}

// root url
if (urlParts.length === 1 && urlParts[0] === "") {
return 0;
}

let weight = -1;
let index = 0;

for (const part of windowUrlParts) {
if (urlParts[index] !== part) {
break;
for (const part of urlParts) {
if (windowUrlParts[index] !== part) {
// Ex: windowURl: /get-started/designers should match to a menu "/#/get-started, but not match to "/get-started/developers
// So if we check by menu (linkParts) and have anything not matched, it should return -1 (not matched), otherwise menu /get-started/developers & menu /get-started/ will have the same weight
return -1;
}
weight += 1;
index++;
Expand All @@ -34,54 +31,12 @@ export function isUrlMatch(windowUrl: URL | Location, testUrl: string): number {
return weight >= 0 ? weight + 1 : weight;
}

function findMaxIndexMatchedToWindowUrlParts(windowUrlParts: string[], urlParts: string[]) {

for (let urlPartsIndex = 0; urlPartsIndex < urlParts.length; urlPartsIndex++) {
for (let windowUrlPartsIndex = 0; windowUrlPartsIndex < windowUrlParts.length; windowUrlPartsIndex++) {
const cleanedWindowUrlPart = windowUrlParts[windowUrlPartsIndex].split("#")[0];
const cleanedUrlPart = urlParts[urlPartsIndex].split("#")[0];
if (cleanedUrlPart === cleanedWindowUrlPart) {
return windowUrlPartsIndex;
}
}
}
return -1;
}

function getUrlWeight(windowUrl: string, linkHref: string) {
const windowParts = decodeURIComponent(windowUrl).replace(/^\/#?/, "").split("/");
const linkParts = decodeURIComponent(linkHref).replace(/^\//, "").split("/");



let startIndex = findMaxIndexMatchedToWindowUrlParts(windowParts, linkParts);
if (startIndex === -1) {
return -1;
}
// Weight should start with matched index on windowUrl. Ex: window.pathname="/ui-components/#/", linkHref="#/", Home menu should have higher weight than the rest
let weight = startIndex;

for (let i = 0; i < linkParts.length && startIndex < windowParts.length; i++) {
const cleanedWindowPartStr = windowParts[startIndex].split("#")[0];
const cleanedLinkPartStr = linkParts[i].split("#")[0];
if (cleanedWindowPartStr === cleanedLinkPartStr) {
// Increase weight for each matching segment
weight += 1;
} else {
// Break loop on first non-match
break;
}
startIndex++;
}

return weight;
}

export function getMatchedLink(links: Element[], windowUrl: string) {
export function getMatchedLink(links: Element[], windowUrl: URL | Location) {
const weights = links.map((link) => {
if (link.getAttribute("target")) return -1;
return getUrlWeight(
windowUrl,
return isUrlMatch(
windowUrl,
(link as HTMLLinkElement).getAttribute("href") || "",
)
}
Expand Down
26 changes: 13 additions & 13 deletions libs/web-components/src/components/app-header-menu/AppHeaderMenu.svelte
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<svelte:options customElement="goa-app-header-menu" />
<script lang="ts" context="module">
export type GoAAppHeaderMenuProps = {
export type AppHeaderMenuProps = {
el: HTMLElement,
links: Element[],
currentHref?: string,
};
</script>
<script lang="ts">
import { onDestroy, onMount, tick } from "svelte";
import { validateRequired } from "../../common/utils";
import { onMount, tick } from "svelte";
import {getSlottedChildren, validateRequired} from "../../common/utils";
import type { GoAIconType } from "../icon/Icon.svelte";
import { TABLET_BP } from "../../common/breakpoints";
Expand Down Expand Up @@ -54,20 +54,19 @@
function dispatchInit() {
if (!_slotParentEl) return;
const slottedChildren = getSlottedChildren(_slotParentEl);
const slot = _slotParentEl.querySelector("slot") as HTMLSlotElement;
if (!slot) return;
if (slottedChildren.length === 0) return;
const links = slot
.assignedElements()
const links = slottedChildren
.filter((el) => el.tagName === "A")
.map((el) => {
el.classList.remove("current");
return el;
});
setTimeout(() => {
_rootEl?.dispatchEvent(new CustomEvent<GoAAppHeaderMenuProps>("appheadermenu:mounted", {
_rootEl?.dispatchEvent(new CustomEvent<AppHeaderMenuProps>("appheadermenu:mounted", {
detail: {
el: _rootEl,
links: links
Expand All @@ -88,11 +87,10 @@
function setCurrentLink(href: string) {
if (!_slotParentEl) return;
const slot = _slotParentEl.querySelector("slot") as HTMLSlotElement;
if (!slot) return;
const slotChildren = getSlottedChildren(_slotParentEl);
if (slotChildren.length === 0) return;
const links = slot
.assignedElements()
const links = slotChildren
.filter((el) => el.tagName === "A")
.map((el) => {
el.classList.remove("current");
Expand Down Expand Up @@ -148,7 +146,9 @@

<svelte:window bind:innerWidth={_innerWidth} />

<div bind:this={_rootEl}>
<div
bind:this={_rootEl}
data-testid="rootEl">
{#if _desktop}
<goa-popover
bind:this={_popoverEl}
Expand Down
37 changes: 37 additions & 0 deletions libs/web-components/src/components/app-header-menu/app-header-menu.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,43 @@ describe("Desktop", () => {
})
});

it("listen to appheader:current:change event and set link to be active", async () => {
const rootEl = screen.queryByTestId("rootEl");

// Listen to parent GoAAppHeader to dispatch event current:change
rootEl?.dispatchEvent(new CustomEvent("appheader:current:change", {
detail: "#seniors"
}));

await waitFor(() => {
const currentLink = $("a.current");
expect(currentLink?.getAttribute("href")).toBe("#seniors");
// We should make sure when router link is changed, the app-header-menu is closed
const popover = $("goa-popover");
expect( popover.getAttribute("open")).toBe("false");
})

// When parent dispatch event with empty link, means no link should be highlighted, we should remove the current class
rootEl?.dispatchEvent(new CustomEvent("appheader:current:change", {
detail: ""
}));

await waitFor(() => {
const currentLink = $("a.current");
expect(currentLink).toBeNull();
});

// When parent dispatch an event with parent's link, means no link under app-header-menu should be highlighted
rootEl?.dispatchEvent(new CustomEvent("appheader:current:change", {
detail: "parent-link"
}));

await waitFor(() => {
const currentLink = $("a.current");
expect(currentLink).toBeNull();
});
});

it("close the menu if the link handles other function beside navigate to new page", async () => {
const specialLink = $("a[href='#special']");
await user.click(specialLink);
Expand Down
Loading

0 comments on commit 6300043

Please sign in to comment.