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

chore(header): changed path matching foractive-route prop to also work for relative paths #4396

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions .changeset/tall-rabbits-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@swisspost/internet-header': minor
---

Changed how the matching for the `active-route` prop works by allowing for usage of relative paths.
56 changes: 29 additions & 27 deletions packages/internet-header/src/services/route.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
ensureUrlWithOrigin,
compareRoutes,
compileScoreList,
getSimilarityScore,
Expand Down Expand Up @@ -50,46 +51,47 @@ describe('route.service.ts', () => {
});

describe('compareRoutes', () => {
const post = new URL('https://post.ch');
const letters = new URL('https://post.ch/briefe');
const deep = new URL('https://post.ch/briefe/inland');
const search = new URL('https://post.ch/briefe?q=search');
const hash = new URL('https://post.ch/briefe#hash');
const nope = new URL('https://post.de/briefe');
const upper = new URL('https://post.ch/Briefe');
const urls = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have some tests for relative path URLs as well

post: ensureUrlWithOrigin('https://post.ch'),
letters: ensureUrlWithOrigin('https://post.ch/briefe'),
deep: ensureUrlWithOrigin('https://post.ch/briefe/inland'),
search: ensureUrlWithOrigin('https://post.ch/briefe?q=search'),
hash: ensureUrlWithOrigin('https://post.ch/briefe#hash'),
nope: ensureUrlWithOrigin('https://post.de/briefe'),
upper: ensureUrlWithOrigin('https://post.ch/Briefe'),
};

it('Correctly scores routes in auto mode', () => {
// Left: current browser URL, right: URL in Nav
expect(compareRoutes(letters, post, 'auto')).toBe(1);
expect(compareRoutes(deep, letters, 'auto')).toBe(2);
expect(compareRoutes(search, letters, 'auto')).toBe(Infinity);
expect(compareRoutes(search, hash, 'auto')).toBe(Infinity);
expect(compareRoutes(letters, deep, 'auto')).toBe(0);
expect(compareRoutes(nope, letters, 'auto')).toBe(0);
expect(compareRoutes(letters, upper, 'auto')).toBe(Infinity);
expect(compareRoutes(deep, upper, 'auto')).toBe(2);
expect(compareRoutes(urls.letters, urls.post, 'auto')).toBe(1);
expect(compareRoutes(urls.deep, urls.letters, 'auto')).toBe(2);
expect(compareRoutes(urls.search, urls.letters, 'auto')).toBe(Infinity);
expect(compareRoutes(urls.search, urls.hash, 'auto')).toBe(Infinity);
expect(compareRoutes(urls.letters, urls.deep, 'auto')).toBe(0);
expect(compareRoutes(urls.nope, urls.letters, 'auto')).toBe(0);
expect(compareRoutes(urls.letters, urls.upper, 'auto')).toBe(Infinity);
expect(compareRoutes(urls.deep, urls.upper, 'auto')).toBe(2);
});

it('Correctly scores routes in exact mode', () => {
// Left: current browser URL, right: URL in Nav
expect(compareRoutes(post, letters)).toBe(0);
expect(compareRoutes(letters, deep)).toBe(0);
expect(compareRoutes(letters, search)).toBe(Infinity);
expect(compareRoutes(hash, search)).toBe(Infinity);
expect(compareRoutes(deep, letters)).toBe(0);
expect(compareRoutes(letters, nope)).toBe(0);
expect(compareRoutes(nope, letters)).toBe(0);
expect(compareRoutes(letters, nope, 'exact')).toBe(0);
expect(compareRoutes(letters, upper, 'exact')).toBe(Infinity);
expect(compareRoutes(urls.post, urls.letters, 'exact')).toBe(0);
expect(compareRoutes(urls.letters, urls.deep, 'exact')).toBe(0);
expect(compareRoutes(urls.letters, urls.search, 'exact')).toBe(Infinity);
expect(compareRoutes(urls.hash, urls.search, 'exact')).toBe(Infinity);
expect(compareRoutes(urls.deep, urls.letters, 'exact')).toBe(0);
expect(compareRoutes(urls.letters, urls.nope, 'exact')).toBe(0);
expect(compareRoutes(urls.nope, urls.letters, 'exact')).toBe(0);
expect(compareRoutes(urls.letters, urls.nope, 'exact')).toBe(0);
expect(compareRoutes(urls.letters, urls.upper, 'exact')).toBe(Infinity);
});

it('Does not fail on invalid arguments', () => {
// @ts-expect-error
expect(compareRoutes(null, nope, 'auto')).toBe(0);
expect(compareRoutes(null, urls.nope, 'auto')).toBe(0);
// @ts-expect-error
expect(compareRoutes(null, undefined, 'auto')).toBe(0);
// @ts-expect-error
expect(compareRoutes(post, nope, null)).toBe(0);
expect(compareRoutes(urls.post, urls.nope, null)).toBe(0);
});
});

Expand Down
23 changes: 21 additions & 2 deletions packages/internet-header/src/services/route.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const markActiveRoute = (
compareUrl = new URL(window.location.href);
} else {
try {
compareUrl = new URL(activeRouteProp, document.location.origin);
compareUrl = ensureUrlWithOrigin(activeRouteProp);
} catch (error) {
console.warn(
`Active Route: ${activeRouteProp} is not a valid URL. Navigation highlighting has been disabled.`,
Expand All @@ -52,6 +52,20 @@ export const markActiveRoute = (
return config;
};

/**
* Ensure URL has an origin by adding current origin if needed
* @param url URL or path string
* @returns URL with origin
*/
export const ensureUrlWithOrigin = (url: string): URL => {
try {
return new URL(url);
} catch {
// If URL construction fails, it's likely relative, so prepend origin
return new URL(url, document.location.origin);
}
};

/**
* Check if the portal config set any active route
* @param config Main navigation config
Expand Down Expand Up @@ -183,7 +197,12 @@ export const compareRoutes = (
matchMode?: 'auto' | 'exact',
): number => {
// One url is not defined or they don't share the same orign
if (baseUrl === null || compareUrl === null || baseUrl.origin !== compareUrl.origin) {
if (baseUrl === null || compareUrl === null) {
return 0;
}

const normalizeOrigin = (origin: string) => origin.replace('www.', '');
if (normalizeOrigin(baseUrl.origin) !== normalizeOrigin(compareUrl.origin)) {
return 0;
}

Expand Down
Loading