From 81f8a2c9580ec07dd17e2f596b9b1b50fae8237a Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Sun, 7 Jul 2024 18:46:17 +0200 Subject: [PATCH] Fixes an edge case in custom pagination link processing (#2105) Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com> --- .changeset/small-kangaroos-smile.md | 8 ++ .../__tests__/basics/navigation.test.ts | 2 +- .../basics/pagination-with-base.test.ts | 80 +++++++++++++++++++ packages/starlight/utils/navigation.ts | 30 +++++-- 4 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 .changeset/small-kangaroos-smile.md create mode 100644 packages/starlight/__tests__/basics/pagination-with-base.test.ts diff --git a/.changeset/small-kangaroos-smile.md b/.changeset/small-kangaroos-smile.md new file mode 100644 index 00000000000..ca882622ab4 --- /dev/null +++ b/.changeset/small-kangaroos-smile.md @@ -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. diff --git a/packages/starlight/__tests__/basics/navigation.test.ts b/packages/starlight/__tests__/basics/navigation.test.ts index 34ccb434fa9..4492c95c193 100644 --- a/packages/starlight/__tests__/basics/navigation.test.ts +++ b/packages/starlight/__tests__/basics/navigation.test.ts @@ -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: {}, diff --git a/packages/starlight/__tests__/basics/pagination-with-base.test.ts b/packages/starlight/__tests__/basics/pagination-with-base.test.ts new file mode 100644 index 00000000000..6619a57b179 --- /dev/null +++ b/packages/starlight/__tests__/basics/pagination-with-base.test.ts @@ -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; + + 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; + + 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); + }); +}); diff --git a/packages/starlight/utils/navigation.ts b/packages/starlight/utils/navigation.ts index 85829f53977..987e89e40ce 100644 --- a/packages/starlight/utils/navigation.ts +++ b/packages/starlight/utils/navigation.ts @@ -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. */ @@ -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, @@ -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. */ @@ -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, @@ -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.