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.

Sign up for GitHub

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

Jump to bottom

🗂 Support url slugs with multiple path segments #489

Merged
merged 8 commits into from
Oct 24, 2024
Merged

🗂 Support url slugs with multiple path segments #489

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3700c61
🗂 Support url slugs with multiple path segments
fwkoch Oct 23, 2024
8714cd4
Add url_folders option to template.yml
fwkoch Oct 23, 2024
f52e345
Replace all slug dots with slashes, not just first
fwkoch Oct 24, 2024
83a9310
Sitemap
rowanc1 Oct 24, 2024
8da5b4c
code review
rowanc1 Oct 24, 2024
f8cd1ae
Move to folders
rowanc1 Oct 24, 2024
c2d115a
upgrade myst
rowanc1 Oct 24, 2024
2cb7e55
Update themes/article/app/utils/loaders.server.ts
rowanc1 Oct 24, 2024
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
8 changes: 8 additions & 0 deletions .changeset/early-plums-double.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@myst-theme/common': patch
'@myst-theme/article': patch
'@myst-theme/site': patch
'@myst-theme/book': patch
---

Support url slugs with multiple path segments
3 changes: 2 additions & 1 deletion packages/common/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ export function getProjectHeadings(
},
...project.pages.map((p) => {
if (!('slug' in p)) return p;
const slug = p.slug?.replace(/\.index$/, '').replaceAll('.', '/');
rowanc1 marked this conversation as resolved.
Show resolved Hide resolved
return {
...p,
path: projectSlug && project.slug ? `/${project.slug}/${p.slug}` : `/${p.slug}`,
path: projectSlug && project.slug ? `/${project.slug}/${slug}` : `/${slug}`,
};
}),
];
Expand Down
2 changes: 1 addition & 1 deletion packages/site/src/components/Navigation/Navigation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const ConfigurablePrimaryNavigation = ({

if (children)
console.warn(
`Including children in Navigation can break keyboard accessbility and is deprecated. Please move children to the page component.`,
`Including children in Navigation can break keyboard accessibility and is deprecated. Please move children to the page component.`,
);

// the logic on the following line looks wrong, this will return `null` or `<></>`
Expand Down
10 changes: 8 additions & 2 deletions packages/site/src/components/Navigation/TableOfContentsItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,16 @@ function nestToc(toc: Heading[]): NestedHeading[] {
return items;
}

function pathnameMatchesHeading(pathname: string, heading: Heading, baseurl?: string) {
const headingPath = withBaseurl(heading.path, baseurl);
if (pathname && headingPath === `${pathname}/index`) return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a few small changes in here that allow index pages to be accessible at the folder-path url.

For example, the file folder1/folder2/index.md is available at /folder1/folder2 (and /folder1/folder2/index redirects back to /folder1/folder2).

return headingPath === pathname;
}

function childrenOpen(headings: NestedHeading[], pathname: string, baseurl?: string): string[] {
return headings
.map((heading) => {
if (withBaseurl(heading.path, baseurl) === pathname) return [heading.id];
if (pathnameMatchesHeading(pathname, heading, baseurl)) return [heading.id];
const open = childrenOpen(heading.children, pathname, baseurl);
if (open.length === 0) return [];
return [heading.id, ...open];
Expand Down Expand Up @@ -110,7 +116,7 @@ const NestedToc = ({ heading }: { heading: NestedHeading }) => {
useEffect(() => {
if (nav.state === 'idle') setOpen(startOpen);
}, [nav.state]);
const exact = pathname === withBaseurl(heading.path, baseurl);
const exact = pathnameMatchesHeading(pathname, heading, baseurl);
if (!heading.children || heading.children.length === 0) {
return (
<LinkItem
Expand Down
5 changes: 4 additions & 1 deletion packages/site/src/seo/sitemap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ export function getSiteSlugs(
const projectSlug = project.slug ? `/${project.slug}` : '';
const pages = project.pages
.filter((page): page is ManifestProjectItem => 'slug' in page)
.map((page) => `${baseurl}${projectSlug}/${page.slug}`);
.map(
(page) =>
`${baseurl}${projectSlug}/${page.slug?.replace(/\.index$/, '').replace(/\./g, '/')}`,
);
if (opts?.excludeIndex) return [...pages];
return [
opts?.explicitIndex
Expand Down
11 changes: 6 additions & 5 deletions themes/article/app/routes/$.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@ export const meta: V2_MetaFunction<typeof loader> = ({ data, matches, location }
export const links: LinksFunction = () => [KatexCSS];

export const loader: LoaderFunction = async ({ params, request }) => {
const [first, second] = new URL(request.url).pathname.slice(1).split('/');
const projectName = second ? first : undefined;
const slug = second || first;
const [first, ...rest] = new URL(request.url).pathname.slice(1).split('/');
const config = await getConfig();
const project = getProject(config, projectName ?? slug);
const project = getProject(config, first);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I learned about some strangeness in the article theme while making these changes: the article theme does not work correctly with project slugs. I think it is ok if project slugs just do not work with article theme, but we should make that more clear. For example, here we should not even try to load the project based on the URL; it should always just be the default single project.

However, for this PR, I tried as best as I could to keep parity with the old functionality.

const projectName = project?.slug === first ? first : undefined;
const slugParts = projectName ? rest : [first, ...rest];
const slug = slugParts.length ? slugParts.join('.') : undefined;
const flat = isFlatSite(config);
const page = await getPage(request, {
project: flat ? projectName : projectName ?? slug,
project: flat ? projectName : (projectName ?? slug),
slug: flat ? slug : projectName ? slug : undefined,
redirect: process.env.MODE === 'static' ? false : true,
});
Expand Down
10 changes: 7 additions & 3 deletions themes/article/app/routes/$slug[.json].tsx → ...utes/($a).($b).($c).($d).$slug[.json].tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,27 @@ function api404(message = 'No API route found at this URL') {
}

export const loader: LoaderFunction = async ({ request, params }) => {
const { slug } = params;
const [first, ...rest] = new URL(request.url).pathname
.slice(1)
.replace(/\.json$/, '')
.split('/');
// Handle /myst.xref.json as slug
if (slug === 'myst.xref') {
if (rest.length === 0 && first === 'myst.xref') {
const xref = await getMystXrefJson();
if (!xref) {
return json({ message: 'myst.xref.json not found', status: 404 }, { status: 404 });
}
return json(xref);
}
// Handle /myst.search.json as slug
else if (slug === 'myst.search') {
else if (rest.length === 0 && first === 'myst.search') {
const search = await getMystSearchJson();
if (!search) {
return json({ message: 'myst.search.json not found', status: 404 }, { status: 404 });
}
return json(search);
}
const slug = [first, ...rest].join('.');
const data = await getPage(request, { slug }).catch(() => null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Look at this for example - on this route, we do not mention anything about project. The other route should probably be the same way.)

if (!data) return api404('No page found at this URL.');
return json(data, {
Expand Down
16 changes: 12 additions & 4 deletions themes/article/app/utils/loaders.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,19 @@ export async function getPage(
const project = getProject(config, projectName);
if (!project) throw responseNoArticle();
if (opts.slug === project.index && opts.redirect) {
return redirect(projectName ? `/${projectName}` : '/');
throw redirect(projectName ? `/${projectName}` : '/');
}
if (opts.slug?.endsWith('.index') && opts.redirect) {
const newSlug = opts.slug.slice(0, -6);
throw redirect(projectName ? `/${projectName}/${newSlug}` : `/${newSlug}`);
}
let slug = opts.loadIndexPage || opts.slug == null ? project.index : opts.slug;
let loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) {
rowanc1 marked this conversation as resolved.
Show resolved Hide resolved
slug = `${slug}.index`;
loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) throw responseNoArticle();
}
const slug = opts.loadIndexPage || opts.slug == null ? project.index : opts.slug;
const loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) throw responseNoArticle();
const footer = getFooterLinks(config, projectName, slug);
return { ...loader, footer, domain: getDomainFromRequest(request), project: projectName };
}
Expand Down
3 changes: 3 additions & 0 deletions themes/article/template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ options:
- type: boolean
id: numbered_references
description: Show references as numbered, rather than in APA-style. Only applies to parenthetical citations.
- type: boolean
id: url_folders
description: Respect nested folder structure in URL paths.
build:
install: npm install
start: npm run start
Expand Down
9 changes: 5 additions & 4 deletions themes/book/app/routes/$.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,12 @@ export const meta: V2_MetaFunction<typeof loader> = ({ data, matches, location }
export const links: LinksFunction = () => [KatexCSS];

export const loader: LoaderFunction = async ({ params, request }) => {
const [first, second] = new URL(request.url).pathname.slice(1).split('/');
const projectName = second ? first : undefined;
const slug = second || first;
const [first, ...rest] = new URL(request.url).pathname.slice(1).split('/');
const config = await getConfig();
const project = getProject(config, projectName ?? slug);
const project = getProject(config, first);
const projectName = project?.slug === first ? first : undefined;
const slugParts = projectName ? rest : [first, ...rest];
const slug = slugParts.length ? slugParts.join('.') : undefined;
const flat = isFlatSite(config);
const page = await getPage(request, {
project: flat ? projectName : (projectName ?? slug),
Expand Down
17 changes: 10 additions & 7 deletions ...k/app/routes/($project)_.$slug[.json].tsx → ...ct)_.($a).($b).($c).($d).$slug[.json].tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ function api404(message = 'No API route found at this URL') {
}

export const loader: LoaderFunction = async ({ request, params }) => {
const { project, slug } = params;
const [first, ...rest] = new URL(request.url).pathname
.slice(1)
.replace(/\.json$/, '')
.split('/');
// Handle /myst.xref.json as slug
if (project === undefined && slug === 'myst.xref') {
if (rest.length === 0 && first === 'myst.xref') {
const xref = await getMystXrefJson();
if (!xref) {
return json({ message: 'myst.xref.json not found', status: 404 }, { status: 404 });
}
return json(xref);
}
// Handle /myst.search.json as slug
else if (slug === 'myst.search') {
else if (rest.length === 0 && first === 'myst.search') {
const search = await getMystSearchJson();
if (!search) {
return json({ message: 'myst.search.json not found', status: 404 }, { status: 404 });
Expand All @@ -33,10 +36,10 @@ export const loader: LoaderFunction = async ({ request, params }) => {
}
const config = await getConfig();
const flat = isFlatSite(config);
const data = await getPage(request, {
project: flat ? project : (project ?? slug),
slug: flat ? slug : project ? slug : undefined,
});
const project = flat ? undefined : first;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the book theme, all the project slug stuff works correctly.

const slugParts = flat ? [first, ...rest] : rest;
const slug = slugParts.join('.');
const data = await getPage(request, { project, slug });
if (!data) return api404('No page found at this URL.');
return json(data, {
headers: {
Expand Down
18 changes: 13 additions & 5 deletions themes/book/app/utils/loaders.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@myst-theme/common';
import { redirect } from '@remix-run/node';
import { responseNoArticle, responseNoSite, getDomainFromRequest } from '@myst-theme/site';
import type { MystSearchIndex } from '@myst-theme/search';
import type { MystSearchIndex } from '@myst-theme/search';

const CONTENT_CDN_PORT = process.env.CONTENT_CDN_PORT ?? '3100';
const CONTENT_CDN = process.env.CONTENT_CDN ?? `http://localhost:${CONTENT_CDN_PORT}`;
Expand Down Expand Up @@ -63,11 +63,19 @@ export async function getPage(
const project = getProject(config, projectName);
if (!project) throw responseNoArticle();
if (opts.slug === project.index && opts.redirect) {
return redirect(projectName ? `/${projectName}` : '/');
throw redirect(projectName ? `/${projectName}` : '/');
}
if (opts.slug?.endsWith('.index') && opts.redirect) {
const newSlug = opts.slug.replace(/\.index$/, '').replace(/\./g, '/');
throw redirect(projectName ? `/${projectName}/${newSlug}` : `/${newSlug}`);
}
let slug = opts.loadIndexPage || opts.slug == null ? project.index : opts.slug;
let loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) {
slug = `${slug}.index`;
loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) throw responseNoArticle();
}
const slug = opts.loadIndexPage || opts.slug == null ? project.index : opts.slug;
const loader = await getStaticContent(projectName, slug).catch(() => null);
if (!loader) throw responseNoArticle();
const footer = getFooterLinks(config, projectName, slug);
return { ...loader, footer, domain: getDomainFromRequest(request), project: projectName };
}
Expand Down
3 changes: 3 additions & 0 deletions themes/book/template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ options:
- type: boolean
id: numbered_references
description: Show references as numbered, rather than in APA-style. Only applies to parenthetical citations.
- type: boolean
id: url_folders
description: Respect nested folder structure in URL paths.
build:
install: npm install
start: npm run start
Expand Down
Loading