Skip to content

Commit

Permalink
fix(#1386): AppHeader not updating current selection when using route…
Browse files Browse the repository at this point in the history
…rLink
  • Loading branch information
vanessatran-ddi committed May 14, 2024
1 parent 81212e3 commit c2c5e84
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 19 deletions.
78 changes: 77 additions & 1 deletion libs/web-components/src/common/urls.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isUrlMatch } from "./urls";
import {getMatchedLink, isUrlMatch} from "./urls";
import { it } from "vitest";

interface MyTest {
Expand Down Expand Up @@ -124,3 +124,79 @@ it("should match urls", async () => {
expect(isUrlMatch(spec.windowUrl, spec.testUrl)).toEqual(spec.weight);
}
});

describe("should getMatchedLink", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const links: any[] = [
{
getAttribute: (attr: string) => attr === 'href' ? "#/" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "#/get-started" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "#/tabs" : null,
},
{
getAttribute: (attr: string) => attr === 'href' ? "/patterns" : null,
},
{
getAttribute: (attr: string) => {
if (attr === 'href') return "https://google.com/choose";
if (attr === 'target') return "_blank";
return null;
}
}
];


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");
});

})
63 changes: 63 additions & 0 deletions libs/web-components/src/common/urls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,66 @@ export function isUrlMatch(windowUrl: URL | Location, testUrl: string): number {
// if weight was incremented once, then it actually has a value of 1, not 0
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) {
const weights = links.map((link) => {
if (link.getAttribute("target")) return -1;
return getUrlWeight(
windowUrl,
(link as HTMLLinkElement).getAttribute("href") || "",
)
}
);
const maxWeight = Math.max(...weights);
if (weights.filter(weight => weight === maxWeight).length > 1) {
// // If all weights are the same or duplicated and there are multiple links, return null
return null;
}
const indexOfMaxWeight = weights.indexOf(maxWeight);
if (indexOfMaxWeight > -1) {
return links[indexOfMaxWeight];
}

return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@
}
button:focus {
outline: var(--goa-border-width-l) solid
var(--goa-color-interactive-focus);
var(--goa-color-interactive-focus);
outline-offset: -3px;
}
.heading {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ import AppHeaderMenuWrapper from "./AppHeaderMenuWrapper.test.svelte"
import { render, waitFor, screen } from "@testing-library/svelte"
import userEvent from "@testing-library/user-event"
import type { UserEvent } from "@testing-library/user-event/dist/types/setup/setup";
import { it, describe } from "vitest";
import { it, describe, vi } from "vitest";
import {tick} from "svelte";
import {isUrlMatch} from "../../common/urls";

vi.mock('../../common/urls', () => ({
...vi.importActual('../../common/urls'),
isUrlMatch: vi.fn(),
}));

let user: UserEvent;

Expand Down Expand Up @@ -67,7 +73,7 @@ describe("Desktop", () => {
expect( popover.getAttribute("open")).toBe("false");
// Functionality is handled as usual
const text = await screen.findByTestId("test-without-loading");
expect((await text).innerHTML).toBe("Test without loading");
expect(text.innerHTML).toBe("Test without loading");
});

})
Expand All @@ -90,6 +96,13 @@ describe("Mobile", () => {
leadingicon: "add",
})

// Make sure onRouteChange isn't trigger
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
isUrlMatch.mockImplementation((location, currentLocation) => {
return location.hash === currentLocation;
});

const c = result.container
$ = c.querySelector.bind(c);
$$ = c.querySelectorAll.bind(c);
Expand Down Expand Up @@ -122,6 +135,7 @@ describe("Mobile", () => {
})

it("opens/closes on the space key", async () => {

const btn = $("button");

btn.focus()
Expand Down Expand Up @@ -161,10 +175,11 @@ describe("Mobile", () => {

it("focuses on the links on `tab`", async () => {
const btn = $("button");

await user.tab();
btn.focus()
await user.keyboard("{enter}")
await waitFor(async () => {
screen.debug();
const links = $$("a");
expect(links.length).toBe(4);

Expand Down
36 changes: 24 additions & 12 deletions libs/web-components/src/components/app-header/AppHeader.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<script lang="ts">
import { onDestroy, onMount, tick } from "svelte";
import { MOBILE_BP, TABLET_BP } from "../../common/breakpoints";
import { isUrlMatch, getMatchedLink } from "../../common/urls";
// optional
export let heading: string = "";
Expand Down Expand Up @@ -43,39 +44,51 @@
// Hooks
onMount(() => {
window.addEventListener("popstate", onRouteChange, true);
onMount(async() => {
await tick();
setCurrentLink();
addEventListeners();
});
onDestroy(() => {
window.removeEventListener("popstate", onRouteChange, true);
});
function addEventListeners() {
// watch path changes
let currentLocation = document.location.href;
const observer = new MutationObserver((_mutationList) => {
if (isUrlMatch(document.location, currentLocation)) {
currentLocation = document.location.href;
onRouteChange();
}
});
observer.observe(document.body, { childList: true, subtree: true });
window.addEventListener("popstate", onRouteChange, true);
}
function onRouteChange() {
setCurrentLink();
hideMenu();
}
// Update component if the current browser url matches one of this element's child links
function setCurrentLink() {
if (!_slotParentEl) return;
const slot = _slotParentEl.querySelector("slot") as HTMLSlotElement;
if (!slot) return;
const link = slot
const url = `${window.location.pathname}${window.location.search}${window.location.hash}`;
const links = slot
.assignedElements()
.filter((el) => el.tagName === "A")
.map((el) => {
el.classList.remove("current");
return el;
})
.find((el) => {
const href = (el as HTMLLinkElement).href;
const url = `${window.location.pathname}${window.location.search}${window.location.hash}`;
return href.endsWith(url);
});
const link = getMatchedLink(links, url);
if (link) {
link.classList.add("current");
}
Expand Down Expand Up @@ -411,9 +424,8 @@
padding: 0;
}
.desktop :global(::slotted(a:focus-within)),
.desktop :global(::slotted(goa-app-header-menu:focus-within)),
.desktop :global(::slotted(a:hover)),
.desktop :global(::slotted(goa-app-header-menu:hover)) {
.desktop :global(::slotted(a:hover))
{
background: var(--goa-color-greyscale-100);
cursor: pointer;
color: var(--goa-color-interactive-hover);
Expand Down
4 changes: 2 additions & 2 deletions libs/web-components/src/components/tabs/Tabs.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
});
function bindTabs(tabs: Element[]) {
const path = window.location.pathname;
const path = window.location.href;
// create buttons (tabs) for each of the tab contents elements
tabs.forEach((tab, index) => {
Expand All @@ -69,7 +69,7 @@
headingEl.classList.add("tab");
headingEl.textContent = heading;
tabSlug = heading;
}
}
}
tabSlug ||= "tab-" + index;
Expand Down

0 comments on commit c2c5e84

Please sign in to comment.