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

Fix edit links in docs/ #9252

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Conversation

kennedybaird
Copy link
Contributor

@kennedybaird kennedybaird commented Aug 4, 2024

These links haven't been working, I think I simplified a little by making sure we only set isIndexPage on the section homepages.

This comment was marked as resolved.

@kennedybaird
Copy link
Contributor Author

Realised this doesn't solve for updates / blog routes.

Thinking about this more, I would suggest removing the edit button from updates/roadmap/blog posts.

Happy to go either direction however.

@simonswiss
Copy link
Contributor

Nice one — thanks for picking that up!

I think we can fix the remaining ones fairly easily:

  1. For the Blog, you could change the editPath in the <BlogPage> component in app/(site)/blog/[post]/page-client.tsx (line 24) from docs/${params?.post}.md to blog/${params?.post}.md.

  2. updates/roadmap/page.tsx can take an isIndexFile to resolve to the correct edit path.

  3. In the same vein of the changes you made in the PR, the docs/guides/document-field-demo/page.tsx needs to be updated with isIndexPage as well.

Would you like to do these changes well to get this PR merged?

If you don't have time, I can make those changes!

@simonswiss
Copy link
Contributor

I've made the 3 changes to your branch — let me know what you think!

@dcousens dcousens changed the title chore: fix docs editbutton links Fix edit button links in docs/ Aug 5, 2024
@dcousens dcousens changed the title Fix edit button links in docs/ Fix edit links in docs/ Aug 5, 2024
@kennedybaird
Copy link
Contributor Author

Great, LGTM @simonswiss 👍

@dcousens dcousens merged commit 9c27c75 into keystonejs:main Aug 5, 2024
1 check passed
@dcousens dcousens mentioned this pull request Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants