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

feat(theme-default): auto sidebar per route and e2e test setup (close… #22

Closed
wants to merge 0 commits into from

Conversation

nruffing
Copy link
Contributor

@nruffing nruffing commented Jan 21, 2024

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Provide a description in this PR that addresses what the PR is solving. If this PR is going to solve an existing issue, please reference the issue (e.g. close vuepress/core#123).

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Other

Description

The auto sidebar feature of the default theme is quite useful, especially in cases where the pages are autogenerated documentation from something like TypeDoc.

Currently you can only use the auto feature on all pages or manually specify the sidebar for every page. It would be great if you could turn the feature on per page as well.

This PR also contains the initial setup of the e2e tests for the default theme and a couple e2e tests of this new feature.

Screenshots

Before

After

closes vuepress/core#17
closes vuepress/core#19

Copy link
Member

@meteorlxy meteorlxy left a comment

Choose a reason for hiding this comment

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

@nruffing I have pushed some changes directly.

I found that some errors are thrown when clicking the auto sidebar. Please resolve it and better to add corresponding e2e testings for clicking them. Thanks!

image

@Mister-Hope
Copy link
Member

@nruffing I have pushed some changes directly.

I found that some errors are thrown when clicking the auto sidebar. Please resolve it and better to add corresponding e2e testings for clicking them. Thanks!

image

I don't think this is because the code change, see #27 I belived this could be solved by #24 naturally once vuepress/core#1447 is merged upstream

@meteorlxy
Copy link
Member

I see. Let's release core first

@Mister-Hope
Copy link
Member

Mister-Hope commented Jan 24, 2024

I would suggest adding the new option as 'heading` instead of 'auto', as it would be great if we can also support 'structure', see https://theme-hope.vuejs.press/guide/layout/sidebar.html#generate-from-file-structure The docs sidebar is using "structure" keyword.

@nruffing

@nruffing
Copy link
Contributor Author

I would suggest adding the new option as 'heading` instead of 'auto', as it would be great if we can also support 'structure', see https://theme-hope.vuejs.press/guide/layout/sidebar.html#generate-from-file-structure The docs sidebar is using "structure" keyword.

@nruffing

I see. That makes sense. I am going to make that change and reopen this PR.

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