Skip to content

Commit

Permalink
Fixes an edge case in custom pagination link processing (#2105)
Browse files Browse the repository at this point in the history
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
  • Loading branch information
delucis and HiDeoo committed Jul 7, 2024
1 parent 61e223b commit 81f8a2c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 9 deletions.
8 changes: 8 additions & 0 deletions .changeset/small-kangaroos-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@astrojs/starlight': patch
---

Fixes an edge case in custom pagination link processing

Custom link values for `prev`/`next` in page frontmatter are now always used as authored.
Previously this was not the case in some edge cases such as for the first and final pages in the sidebar.
2 changes: 1 addition & 1 deletion packages/starlight/__tests__/basics/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ describe('getPrevNextLinks', () => {
expect(withDefaults.prev).toBeUndefined();
expect(withCustomLinks.prev).toEqual({
type: 'link',
href: '/x',
href: 'x',
label: 'X',
isCurrent: false,
attrs: {},
Expand Down
80 changes: 80 additions & 0 deletions packages/starlight/__tests__/basics/pagination-with-base.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { afterAll, beforeAll, describe, expect, test, vi } from 'vitest';
import type {
getPrevNextLinks as getPrevNextLinksType,
getSidebar as getSidebarType,
} from '../../utils/navigation';

vi.mock('astro:content', async () =>
(await import('../test-utils')).mockedAstroContent({
docs: [
['index.mdx', { title: 'Home Page' }],
['environmental-impact.md', { title: 'Eco-friendly docs' }],
['guides/authoring-content.md', { title: 'Authoring Markdown' }],
['guides/components.mdx', { title: 'Components' }],
['reference/frontmatter.md', { title: 'Frontmatter Reference' }],
],
})
);

describe('without base', async () => {
let getPrevNextLinks: typeof getPrevNextLinksType;
let getSidebar: typeof getSidebarType;
let sidebar: ReturnType<typeof getSidebar>;

beforeAll(async () => {
({ getPrevNextLinks, getSidebar } = await import('../../utils/navigation'));
sidebar = getSidebar('/reference/frontmatter/', undefined);
});

test('pagination links are inferred correctly with no frontmatter', () => {
const links = getPrevNextLinks(sidebar, true, {});
expect(links.prev?.href).toBe('/guides/components/');
expect(links.next?.href).toBeUndefined();
});

test('pagination links are used as authored with custom links in frontmatter', () => {
const prevLink = '/other-page';
const nextLink = '/extra-page';
const links = getPrevNextLinks(sidebar, true, {
prev: { link: prevLink, label: 'Other Page' },
next: { link: nextLink, label: 'Extra Page' },
});
expect(links.prev?.href).toBe(prevLink);
expect(links.next?.href).toBe(nextLink);
});
});

describe('with base', () => {
let getPrevNextLinks: typeof getPrevNextLinksType;
let getSidebar: typeof getSidebarType;
let sidebar: ReturnType<typeof getSidebar>;

beforeAll(async () => {
vi.resetModules();
vi.stubEnv('BASE_URL', '/test-base/');
({ getPrevNextLinks, getSidebar } = await import('../../utils/navigation'));
sidebar = getSidebar('/test-base/reference/frontmatter/', undefined);
});

afterAll(() => {
vi.unstubAllEnvs();
vi.resetModules();
});

test('pagination links are inferred correctly with no frontmatter', () => {
const links = getPrevNextLinks(sidebar, true, {});
expect(links.prev?.href).toBe('/test-base/guides/components/');
expect(links.next?.href).toBeUndefined();
});

test('pagination links are used as authored with custom links in frontmatter', () => {
const prevLink = '/other-page';
const nextLink = '/extra-page';
const links = getPrevNextLinks(sidebar, true, {
prev: { link: prevLink, label: 'Other Page' },
next: { link: nextLink, label: 'Extra Page' },
});
expect(links.prev?.href).toBe(prevLink);
expect(links.next?.href).toBe(nextLink);
});
});
30 changes: 22 additions & 8 deletions packages/starlight/utils/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ function linkFromSidebarLinkItem(
if (locale) href = '/' + locale + href;
}
const label = pickLang(item.translations, localeToLang(locale)) || item.label;
return makeLink(href, label, currentPathname, item.badge, item.attrs);
return makeSidebarLink(href, label, currentPathname, item.badge, item.attrs);
}

/** Create a link entry from an automatic internal link item in user config. */
Expand Down Expand Up @@ -160,11 +160,11 @@ function linkFromInternalSidebarLinkItem(
}
const label =
pickLang(item.translations, localeToLang(locale)) || item.label || entry.entry.data.title;
return makeLink(entry.slug, label, currentPathname, item.badge, item.attrs);
return makeSidebarLink(entry.slug, label, currentPathname, item.badge, item.attrs);
}

/** Create a link entry. */
function makeLink(
/** Process sidebar link options to create a link entry. */
function makeSidebarLink(
href: string,
label: string,
currentPathname: string,
Expand All @@ -174,10 +174,24 @@ function makeLink(
if (!isAbsolute(href)) {
href = formatPath(href);
}

const isCurrent = pathsMatch(encodeURI(href), currentPathname);
return makeLink({ label, href, isCurrent, badge, attrs });
}

return { type: 'link', label, href, isCurrent, badge, attrs: attrs ?? {} };
/** Create a link entry */
function makeLink({
isCurrent = false,
attrs = {},
badge = undefined,
...opts
}: {
label: string;
href: string;
isCurrent?: boolean;
badge?: Badge | undefined;
attrs?: LinkHTMLAttributes | undefined;
}): Link {
return { type: 'link', ...opts, badge, isCurrent, attrs };
}

/** Test if two paths are equivalent even if formatted differently. */
Expand Down Expand Up @@ -240,7 +254,7 @@ function treeify(routes: Route[], baseDir: string): Dir {

/** Create a link entry for a given content collection entry. */
function linkFromRoute(route: Route, currentPathname: string): Link {
return makeLink(
return makeSidebarLink(
slugToPathname(route.slug),
route.entry.data.sidebar.label || route.entry.data.title,
currentPathname,
Expand Down Expand Up @@ -388,7 +402,7 @@ function applyPrevNextLinkConfig(
} else if (config.link && config.label) {
// If there is no link and the frontmatter contains both a URL and a label,
// create a new link.
return makeLink(config.link, config.label, '');
return makeLink({ href: config.link, label: config.label });
}
}
// Otherwise, if the global config is enabled, return the generated link if any.
Expand Down

0 comments on commit 81f8a2c

Please sign in to comment.