Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#1368,#1792,#1772,#1569): fixing link active when router change #1843

Merged
merged 3 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 9 additions & 50 deletions libs/web-components/src/assets/css/components.css
Original file line number Diff line number Diff line change
Expand Up @@ -15,65 +15,24 @@ ul.goa-unordered-list li::marker {
}

/* GoAAppHeader */


/** needed to override a,
a:visited at reset.css */
goa-app-header a,
goa-app-header a:visited {
color: var(--goa-color-text-default);
}

goa-app-header a:focus {
outline: var(--goa-border-width-l) solid var(--goa-color-interactive-focus) !important;
outline-offset: -3px !important;
background: var(--goa-color-greyscale-100) !important;
cursor: pointer !important;
color: var(--goa-color-interactive-hover) !important;
}

goa-app-header a:active {
color: var(--goa-color-text-default) !important;
}

goa-app-header a:hover,
goa-app-header a:visited:hover,
goa-app-header goa-app-header-menu:hover {
background: var(--goa-color-greyscale-100);
cursor: pointer !important;
color: var(--goa-color-interactive-hover);
}

goa-app-header-menu a,
goa-app-header-menu a:visited {
box-shadow: inset 0 1px 0 0 var(--goa-color-greyscale-200);
color: var(--goa-color-text-default);
display: block;
padding: calc((3rem - var(--goa-line-height-3)) / 2) 1rem;
text-decoration: none;
goa-app-header a.current.inside-collapse-menu,
vanessatran-ddi marked this conversation as resolved.
Show resolved Hide resolved
goa-app-header a.current.inside-collapse-menu:hover,
vanessatran-ddi marked this conversation as resolved.
Show resolved Hide resolved
goa-app-header-menu a.current,
goa-app-header-menu a.current:hover {
color: var(--goa-color-text-light);
}

goa-app-header-menu a:first-of-type {
box-shadow: none;
}

goa-app-header-menu a:hover {
background: var(--goa-color-greyscale-100);
}

goa-app-header-menu a:focus-visible {
outline: var(--goa-border-width-l) solid var(--goa-color-interactive-focus);
outline-offset: -3px;
}

goa-app-header a.interactive {
font-weight: var(--goa-font-weight-medium);
text-decoration: underline;
color: var(--goa-color-interactive-default);
}

goa-app-header a.interactive:hover {
color: var(--goa-color-interactive-hover);
}

/* Table */

goa-table table {
Expand All @@ -91,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 Expand Up @@ -127,4 +86,4 @@ goa-table tfoot tr:first-child td {

goa-table tfoot tr:last-child td {
border-bottom: none;
}
}
12 changes: 6 additions & 6 deletions libs/web-components/src/assets/css/reset.css
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 Expand Up @@ -143,4 +143,4 @@ goa-container h1:first-of-type,
goa-container h2:first-of-type,
goa-container h3:first-of-type {
margin-top: 0;
}
}
86 changes: 82 additions & 4 deletions 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 @@ -44,13 +44,13 @@ it("should match urls", async () => {
desc: "empty test url",
windowUrl: new URL("http://localhost/foo"),
testUrl: "",
weight: 0,
weight: -1,
},
vanessatran-ddi marked this conversation as resolved.
Show resolved Hide resolved
{
desc: "root path only match",
windowUrl: new URL("http://localhost"),
testUrl: "/",
weight: 0,
weight: 1,
},
{
desc: "path match",
Expand Down Expand Up @@ -121,6 +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);
}
}
});

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" ? "/get-started" : 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";
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,
},
];

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);
}
}
});
36 changes: 28 additions & 8 deletions libs/web-components/src/common/urls.ts
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 @@ -33,3 +30,26 @@ 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;
}

export function getMatchedLink(links: Element[], windowUrl: URL | Location) {
const weights = links.map((link) => {
if (link.getAttribute("target")) return -1;
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);
ArakTaiRoth marked this conversation as resolved.
Show resolved Hide resolved
if (weights.filter((weight) => weight === maxWeight).length > 1) {
return null;
}

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

return null;
}
Loading
Loading