Skip to content

Conversation

timaschew
Copy link
Contributor

@timaschew timaschew commented Apr 13, 2019

Currently only direct children of a groups are checked (if any of them is active).
This fix checks the children recursively.

Fix #1497

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@timaschew timaschew force-pushed the expand-nested-sidebar-children branch from 6d183ff to 290142a Compare April 13, 2019 10:17
@timaschew timaschew mentioned this pull request Apr 27, 2019
18 tasks
@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

Thanks for your work @timaschew. Can you add some test to validate what you mean please

@flozero flozero added the status: awaiting more context Need more context about this issue label Sep 5, 2019
@timaschew
Copy link
Contributor Author

@f3ltron are you talking about to write a test case (code) or about a demo to show?

Later can be viewed here:
Problem: https://vuepress-fix-demo.herokuapp.com/sidebar-problem/docs/subsec/subsubsec/
Fix: https://vuepress-fix-demo.herokuapp.com/sidebar-fix/docs/subsec/subsubsec/

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

Oh ok i definitely see the improvment now ^^. Just let me some time to review it and i will speak with the others about it. You can start by adding / updating the doc part about it

@flozero flozero self-assigned this Sep 5, 2019
@flozero flozero added status: core team assigned Core team member assigned version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@timaschew
Copy link
Contributor Author

You can start by adding / updating the doc part about it

What do you mean? This is a bug for me so I wouldn't assume it require to change any documentation.

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

I will have a look about it. It's look it was actually not supported by Vuepress. Thank's for the PR

@flozero flozero added type: enhancement Request to enhance an existing feature and removed status: awaiting more context Need more context about this issue labels Sep 7, 2019
@flozero
Copy link
Collaborator

flozero commented Sep 7, 2019

Copy link
Collaborator

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

@timaschew Thanks for this PR 💪 Could you just accept my code suggestions to improve code readability

timaschew and others added 4 commits September 10, 2019 22:17
Co-Authored-By: Franck Abgrall <[email protected]>
Co-Authored-By: Franck Abgrall <[email protected]>
Co-Authored-By: Franck Abgrall <[email protected]>
Co-Authored-By: Franck Abgrall <[email protected]>
@timaschew
Copy link
Contributor Author

Hi @kefranabg thanks for the code review! Sure I've confirmed your suggestions.
Let me know if you expect a rebase/squash by me.

@timaschew timaschew changed the title Fix #1497 Expand nested sidebar groups Fix #1497 Expand nested sidebar groups (ready to be squashed) Sep 10, 2019
@kefranabg kefranabg changed the title Fix #1497 Expand nested sidebar groups (ready to be squashed) fix($theme-default): Expand nested sidebar groups Sep 12, 2019
Copy link
Collaborator

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks @timaschew

@kefranabg kefranabg merged commit eb231bf into vuejs:master Sep 12, 2019
@vue-bot
Copy link

vue-bot commented Sep 12, 2019

Hey @timaschew, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: core team assigned Core team member assigned type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Group in sidebar is not expanded for deep nested children
5 participants