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(prev/next link): fix logic to correspond with sidebar config change #85

Closed
wants to merge 2 commits into from
Closed

fix(prev/next link): fix logic to correspond with sidebar config change #85

wants to merge 2 commits into from

Conversation

Spice-Z
Copy link
Contributor

@Spice-Z Spice-Z commented Sep 13, 2020

Now, all of prev/next link are not shown without each pages's frontmatter setting.
From #74, sidebar config path was changed.
Prev/Next link is generated from sidebar config, so some changes are needed.

@posva
Copy link
Member

posva commented Sep 16, 2020

Can you share a reproduction of the problem?

@Spice-Z
Copy link
Contributor Author

Spice-Z commented Sep 18, 2020

@posva
Reproduction

  1. Set vitepress sidebar config children with link not starting from '/',like this
children: [
            { text: 'Index', link: 'guide/index' },
            { text: 'Getting Started', link: 'guide/getting-started' },
            { text: 'Guide Second', link: 'guide/guide-second' },
          ]
  1. Show page that should content Prev/Next link.
  2. Prev/Next link are not shown!

This occurs because pagePath always starting from /, but sidebar config children are not starting from '/'.
When searching pagePath in sidebar, we should consider this difference.

(BTW, thank you to your Vue.js Global stage! I enjoyed it yesterday! )

@posva
Copy link
Member

posva commented Sep 18, 2020

Thank you! I'm glad you liked my talk 😄

I see what you mean right now. I don't think it's a good idea to write relative links like that in children because a lot of people do not realize that a relative URL takes away the last section of the URL to replace it, some will think it appends to the parent, others would think it changes based on the current location. So I think it's better to use absolute URLs on the sidebar: /guide/index

I think there is also a chance here for us to display some messages in the console to warn users about implicit relative URLs and make them change it to absolute URLs instead

We also need to refactor a little part of the server regarding consuming the theme, so I think it's better to hold on on changes to this for the moment! Thank you!

@posva posva closed this Sep 18, 2020
@Spice-Z Spice-Z deleted the fix/prev-next-link branch September 18, 2020 16:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants