-
Notifications
You must be signed in to change notification settings - Fork 0
feat: use breadcrumbPrefix for breadcrumb urls #1006
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
base: main
Are you sure you want to change the base?
Changes from all commits
729bfd4
b1e02e9
739b872
94e1f3c
12ecd47
f67ebf8
0e0b558
7ef8406
74874ef
98fcb28
3bc6161
91b3ac1
c118129
320cb45
99deab4
d836dc8
8eed9e2
3baad99
c2bc029
efa4de5
42d5146
55a8b96
3ce2e6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| import { describe, expect, it, vi, beforeEach, type Mock } from "vitest"; | ||
| import { resolveBreadcrumbs } from "./resolveBreadcrumbs.ts"; | ||
| import { StreamDocument } from "../types/StreamDocument.ts"; | ||
| import { getDirectoryParents } from "../schema/helpers.ts"; | ||
|
|
||
| vi.mock("../schema/helpers.ts", () => ({ | ||
| getDirectoryParents: vi.fn(), | ||
| })); | ||
|
|
||
| const mockedGetDirectoryParents = getDirectoryParents as unknown as Mock; | ||
|
|
||
| const baseDocument: StreamDocument = { | ||
| name: "123 Test Rd", | ||
| dm_directoryParents_123_locations: [ | ||
| { name: "Directory Root", slug: "index.html" }, | ||
| { name: "US", slug: "us" }, | ||
| { name: "TS", slug: "ts" }, | ||
| { name: "Testville", slug: "testville" }, | ||
| ], | ||
| address: { | ||
| line1: "123 Test Rd", | ||
| city: "Testville", | ||
| region: "TS", | ||
| postalCode: "12345", | ||
| countryCode: "US", | ||
| }, | ||
| locale: "en", | ||
| __: { | ||
| pathInfo: { | ||
| primaryLocale: "en", | ||
| breadcrumbPrefix: "locations", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| mockedGetDirectoryParents.mockReset(); | ||
| }); | ||
|
|
||
| describe("resolveBreadcrumbs", () => { | ||
| it("prefers pathInfo breadcrumbs over directory parents", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting wording, more like uses pathInfo breadcrumbsPrefix if exists? |
||
| const documentWithBreadcrumbs: StreamDocument = baseDocument; | ||
|
|
||
| mockedGetDirectoryParents.mockReturnValue([ | ||
| { name: "Directory Parent", slug: "directory-parent" }, | ||
| ]); | ||
|
|
||
| expect(resolveBreadcrumbs(documentWithBreadcrumbs)).toEqual([ | ||
| { name: "Directory Root", slug: "locations/index.html" }, | ||
| { name: "US", slug: "locations/us" }, | ||
| { name: "TS", slug: "locations/ts" }, | ||
| { name: "Testville", slug: "locations/testville" }, | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("falls back to directory parents when pathInfo breadcrumbs are missing", () => { | ||
| const documentWithoutPathInfo: StreamDocument = { | ||
| ...baseDocument, | ||
| __: {}, | ||
| }; | ||
|
|
||
| mockedGetDirectoryParents.mockReturnValue([ | ||
| { name: "Directory Parent", slug: "directory-parent" }, | ||
| ]); | ||
|
|
||
| expect(resolveBreadcrumbs(documentWithoutPathInfo)).toEqual([ | ||
| { name: "Directory Parent", slug: "directory-parent" }, | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("uses the current page when directory parents are missing but children are present", () => { | ||
| const documentWithChildren: StreamDocument = { | ||
| ...baseDocument, | ||
| __: {}, | ||
| dm_directoryChildren: [{}], | ||
| }; | ||
|
|
||
| mockedGetDirectoryParents.mockReturnValue(undefined); | ||
|
|
||
| expect(resolveBreadcrumbs(documentWithChildren)).toEqual([ | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("returns an empty array when no resolver yields breadcrumbs", () => { | ||
| const documentWithoutNames: StreamDocument = { | ||
| ...baseDocument, | ||
| name: undefined, | ||
| __: {}, | ||
| }; | ||
|
|
||
| mockedGetDirectoryParents.mockReturnValue([ | ||
| { name: "", slug: "directory-parent" }, | ||
| ]); | ||
|
|
||
| expect(resolveBreadcrumbs(documentWithoutNames)).toEqual([]); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { getDirectoryParents } from "../schema/helpers.ts"; | ||
| import { StreamDocument } from "../types/StreamDocument.ts"; | ||
| import { | ||
| resolveBreadcrumbsFromPathInfo, | ||
| BreadcrumbLink, | ||
| } from "./resolveBreadcrumbsFromPathInfo.ts"; | ||
|
|
||
| type BreadcrumbResolver = ( | ||
| streamDocument: StreamDocument | ||
| ) => BreadcrumbLink[] | undefined; | ||
|
|
||
| const resolvers: BreadcrumbResolver[] = [ | ||
| (streamDocument) => resolveBreadcrumbsFromPathInfo(streamDocument), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One is called resolve and one is called build. Should we keep it consistent? |
||
| (streamDocument) => buildBreadcrumbsFromDirectory(streamDocument), | ||
| ]; | ||
|
|
||
| export const resolveBreadcrumbs = ( | ||
| streamDocument: StreamDocument | ||
| ): BreadcrumbLink[] => { | ||
| for (const resolve of resolvers) { | ||
| try { | ||
| const result = resolve(streamDocument); | ||
| if (result && result.length) { | ||
| return result; | ||
| } | ||
| } catch { | ||
| // continue to next resolver | ||
| } | ||
| } | ||
| return []; | ||
| }; | ||
|
|
||
| const buildBreadcrumbsFromDirectory = ( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this one in this file but resolveBreadcrumbsFromPathInfo in its own file? |
||
| streamDocument: StreamDocument | ||
| ): BreadcrumbLink[] | undefined => { | ||
| const directoryParents = getDirectoryParents(streamDocument) || []; | ||
|
|
||
| if (directoryParents.length > 0 || streamDocument.dm_directoryChildren) { | ||
| // append the current and filter out missing or malformed data | ||
| const breadcrumbs = [ | ||
| ...directoryParents, | ||
| { name: streamDocument.name, slug: "" }, | ||
| ].filter((b) => b.name); | ||
|
|
||
| return breadcrumbs.length ? breadcrumbs : undefined; | ||
| } | ||
|
|
||
| return undefined; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { resolveBreadcrumbsFromPathInfo } from "./resolveBreadcrumbsFromPathInfo.ts"; | ||
| import { StreamDocument } from "../types/StreamDocument.ts"; | ||
|
|
||
| const baseDocument: StreamDocument = { | ||
| name: "123 Test Rd", | ||
| dm_directoryParents_123_locations: [ | ||
| { name: "Directory Root", slug: "index.html" }, | ||
| { name: "US", slug: "us" }, | ||
| { name: "TS", slug: "ts" }, | ||
| { name: "Testville", slug: "testville" }, | ||
| ], | ||
| address: { | ||
| line1: "123 Test Rd", | ||
| city: "Testville", | ||
| region: "TS", | ||
| postalCode: "12345", | ||
| countryCode: "US", | ||
| }, | ||
| locale: "en", | ||
| __: { | ||
| pathInfo: { | ||
| primaryLocale: "en", | ||
| breadcrumbPrefix: "locations", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| describe("resolveBreadcrumbsFromPathInfo", () => { | ||
| it("builds breadcrumbs from breadcrumbPrefix for the primary locale without locale prefix", () => { | ||
| const englishDocument = baseDocument; | ||
|
|
||
| expect(resolveBreadcrumbsFromPathInfo(englishDocument)).toEqual([ | ||
| { name: "Directory Root", slug: "locations/index.html" }, | ||
| { name: "US", slug: "locations/us" }, | ||
| { name: "TS", slug: "locations/ts" }, | ||
| { name: "Testville", slug: "locations/testville" }, | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("prefixes breadcrumbs with locale for non-primary locales", () => { | ||
| const spanishDocument: StreamDocument = { | ||
| ...baseDocument, | ||
| locale: "es", | ||
| }; | ||
|
|
||
| expect(resolveBreadcrumbsFromPathInfo(spanishDocument)).toEqual([ | ||
| { name: "Directory Root", slug: "es/locations/index.html" }, | ||
| { name: "US", slug: "es/locations/us" }, | ||
| { name: "TS", slug: "es/locations/ts" }, | ||
| { name: "Testville", slug: "es/locations/testville" }, | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("omits the breadcrumb prefix separator when breadcrumbPrefix is empty", () => { | ||
| const documentWithEmptyPrefix: StreamDocument = { | ||
| ...baseDocument, | ||
| __: { | ||
| pathInfo: { | ||
| ...baseDocument.__?.pathInfo, | ||
| breadcrumbPrefix: "", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| expect(resolveBreadcrumbsFromPathInfo(documentWithEmptyPrefix)).toEqual([ | ||
| { name: "Directory Root", slug: "index.html" }, | ||
| { name: "US", slug: "us" }, | ||
| { name: "TS", slug: "ts" }, | ||
| { name: "Testville", slug: "testville" }, | ||
| { name: "123 Test Rd", slug: "" }, | ||
| ]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { normalizeSlug } from "../slugifier.ts"; | ||
| import { StreamDocument } from "../types/StreamDocument.ts"; | ||
| import { isPrimaryLocale } from "./resolveUrlFromPathInfo.ts"; | ||
|
|
||
| export type BreadcrumbLink = { | ||
| name: string; | ||
| slug: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Builds breadcrumb links from __.pathInfo.breadcrumbPrefix and the | ||
| * dm_directoryParents_* field when available. | ||
| */ | ||
| export const resolveBreadcrumbsFromPathInfo = ( | ||
| streamDocument: StreamDocument | ||
| ): BreadcrumbLink[] | undefined => { | ||
| const breadcrumbPrefix = streamDocument.__?.pathInfo?.breadcrumbPrefix; | ||
| if (typeof breadcrumbPrefix !== "string") { | ||
| return undefined; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just return (without undefined) |
||
| } | ||
|
|
||
| const locale = streamDocument?.locale || streamDocument?.meta?.locale || ""; | ||
| if (!locale) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const includeLocalePrefix = | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want to do the same locale prefix logic. It depends on if Spruce has changed the DM config to have the same urlTemplate across all slug values of the DM or if they have the normal DM config to handle locale. |
||
| !isPrimaryLocale(streamDocument) || | ||
| streamDocument.__?.pathInfo?.includeLocalePrefixForPrimaryLocale; | ||
|
|
||
| const normalizedPrefix = normalizeSlug(breadcrumbPrefix) | ||
| .replace(/\/+/g, "/") | ||
| .replace(/^\/+|\/+$/g, ""); | ||
|
|
||
| const directoryParentsEntry = Object.entries(streamDocument).find( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this use the same getDirectoryParents as the other function?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't you just call getDirectoryParents, iterate through it, and apply the prefix? |
||
| ([key, value]) => | ||
| key.startsWith("dm_directoryParents_") && Array.isArray(value) | ||
| ); | ||
| const directoryParents = directoryParentsEntry?.[1]; | ||
| if (!Array.isArray(directoryParents) || directoryParents.length === 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const crumbs: BreadcrumbLink[] = []; | ||
|
|
||
| for (const parent of directoryParents) { | ||
| if (!parent || typeof parent !== "object") { | ||
| continue; | ||
| } | ||
|
|
||
| const directoryLevelSlug = | ||
| typeof parent.slug === "string" | ||
| ? normalizeSlug(parent.slug) | ||
| .replace(/\/+/g, "/") | ||
| .replace(/^\/+|\/+$/g, "") | ||
| : ""; | ||
| if (!directoryLevelSlug) { | ||
| continue; | ||
| } | ||
|
|
||
| const normalizedSlug = normalizedPrefix | ||
| ? `${normalizedPrefix}/${directoryLevelSlug}` | ||
| : directoryLevelSlug; | ||
| const slug = includeLocalePrefix | ||
| ? `${locale}/${normalizedSlug}` | ||
| : normalizedSlug; | ||
|
|
||
| const name = (typeof parent.name === "string" && parent.name) || slug; | ||
|
|
||
| crumbs.push({ name, slug }); | ||
| } | ||
|
|
||
| if (!crumbs.length) { | ||
| return undefined; | ||
| } | ||
|
|
||
| crumbs.push({ name: streamDocument.name ?? "", slug: "" }); | ||
briantstephan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return crumbs; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for this?