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

🗂 Support url slugs with multiple path segments #489

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

fwkoch
Copy link
Contributor

@fwkoch fwkoch commented Oct 23, 2024

This PR includes the theme updates required to support nested folder structures in URLs. See jupyter-book/mystmd#670

If MyST site content is built with the option url_folders (see jupyter-book/mystmd#1601), the folder structure will be reflected as dot-delimited parts in the page slugs. With this PR, the themes will load the correct page based on slash-delimited URLs.

For example, the file folder1/folder2/page.md will be given the slug folder1.folder2.page and will be available at URL /folder1/folder2/page.

Copy link

changeset-bot bot commented Oct 23, 2024

🦋 Changeset detected

Latest commit: 2cb7e55

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
@myst-theme/common Patch
@myst-theme/article Patch
@myst-theme/site Patch
@myst-theme/book Patch
@myst-theme/providers Patch
@myst-theme/frontmatter Patch
@myst-theme/diagrams Patch
@myst-theme/jupyter Patch
@myst-theme/styles Patch
@myst-theme/icons Patch
@myst-theme/search Patch
@myst-theme/search-minisearch Patch
myst-to-react Patch
myst-demo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -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).

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 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.)

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.

@stevejpurves
Copy link
Contributor

@fwkoch has this been tested with notebooks in the mix, and specifically with notebooks in a subfolder being pages on the site and one or more outputs having been included as a figure? I'm curious about how any changes to the slug and location fields might impact that.

@fwkoch
Copy link
Contributor Author

fwkoch commented Oct 23, 2024

@stevejpurves - I have not tested these with notebook compute. I'll do that. location has not been changed at all. slug is now dot-delimited in JSON files, but if you follow urls using that slug, they should resolve in the same way as if the dots were slashes. Hopefully that means everything still works correctly...? But I'll confirm.

@rowanc1
Copy link
Collaborator

rowanc1 commented Oct 24, 2024

We tested that the nested notebooks load at the correct jupyter path!

packages/common/src/utils.ts Outdated Show resolved Hide resolved
@rowanc1 rowanc1 merged commit 653bd6c into main Oct 24, 2024
2 checks passed
@rowanc1 rowanc1 deleted the feat/folder-structure branch October 24, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants