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 @ArakTaiRoth
vanessatran-ddi authored and ArakTaiRoth committed May 29, 2024
1 parent a0016b7 commit de245a7
Show file tree
Hide file tree
Showing 13 changed files with 614 additions and 453 deletions.
4 changes: 2 additions & 2 deletions libs/web-components/src/assets/css/components.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ goa-app-header a.current.inside-collapse-menu,
goa-app-header a.current.inside-collapse-menu:hover,
goa-app-header-menu a.current,
goa-app-header-menu a.current:hover {
color: var(--goa-color-text-light);
color: var(--goa-color-text-light);
}

goa-app-header-menu a:first-of-type {
Expand All @@ -50,7 +50,7 @@ goa-table td {
border-bottom: 1px solid var(--goa-color-greyscale-200);
}

goa-table[variant=relaxed] td {
goa-table[variant="relaxed"] td {
padding: 1.25rem 1rem 1rem 1rem;
}

Expand Down
10 changes: 5 additions & 5 deletions libs/web-components/src/assets/css/reset.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ ol {
padding-left: 1rem;
}

li>ul,
li>ol,
li>ul,
li>ol {
li > ul,
li > ol,
li > ul,
li > ol {
padding-left: 0;
margin-top: 0;
}
Expand Down Expand Up @@ -110,7 +110,7 @@ h6 {
font-weight: var(--goa-font-weight-bold);
}

h3+h1 {
h3 + h1 {
margin-top: -1rem;
}

Expand Down
128 changes: 65 additions & 63 deletions libs/web-components/src/common/urls.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {getMatchedLink, isUrlMatch} from "./urls";
import { getMatchedLink, isUrlMatch } from "./urls";
import { it } from "vitest";

interface MyTest {
Expand Down 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,82 +121,84 @@ 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,
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";
if (attr === 'target') return "_blank";
if (attr === "href") return "https://google.com/choose";
if (attr === "target") return "_blank";
return null;
}
}
},
},
];

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);
}
}
});
75 changes: 15 additions & 60 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,63 +31,21 @@ 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,
(link as HTMLLinkElement).getAttribute("href") || "",
)
}
);
return isUrlMatch(
windowUrl,
(link as HTMLLinkElement).getAttribute("href") || "",
);
});

// If all weights are the same or duplicated, and there are multiple links, return null.Ex: [1,1,1,-1,-1] we will return null
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
if (weights.filter((weight) => weight === maxWeight).length > 1) {
return null;
}

const indexOfMaxWeight = weights.indexOf(maxWeight);
if (indexOfMaxWeight > -1) {
return links[indexOfMaxWeight];
Expand Down
Loading

0 comments on commit de245a7

Please sign in to comment.