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($theme-default): remove error logs for nested sidebar groups #2191

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

KieranHunt
Copy link
Contributor

@KieranHunt KieranHunt commented Feb 18, 2020

This arbitrarily logged out when nesting was greater than 3.

07:58:13.598 [vuepress] detected a too deep nested sidebar group. 2 2.258cb917.js:1:3364

#2190

Summary

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:

If changing the UI of default theme, please provide the before/after screenshot:

N/A

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

  • Yes
  • No

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

N/A

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

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

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:

Opened an issue here: #2190

This arbitrarily logged out when nesting was greater than 3.

```
07:58:13.598 [vuepress] detected a too deep nested sidebar group. 2 2.258cb917.js:1:3364
```

vuejs#2190
@KieranHunt KieranHunt changed the title Remove unneeded logging for nested sidebar groups Remove unneeded logging for nested sidebar groups fix #2190 Feb 18, 2020
@KieranHunt KieranHunt changed the title Remove unneeded logging for nested sidebar groups fix #2190 fix: Remove unneeded logging for nested sidebar groups Feb 18, 2020
@KieranHunt KieranHunt requested a review from ulivz February 18, 2020 06:15
@KieranHunt
Copy link
Contributor Author

Added @ulivz, they seem to be the author of the original change here: 01dd45b

@kefranabg
Copy link
Collaborator

Hi @KieranHunt I think this is because initially, VuePress was not supporting nested sidebar items indentation if it was higher than 3. But it supposed to be fixed with #1973.

Could you confirm the indentation is correct when nesting is more than 3.

@kefranabg
Copy link
Collaborator

Also could you fix parts of the docs saying the maximum is 3?

@KieranHunt
Copy link
Contributor Author

Could you confirm the indentation is correct when nesting is more than 3.

Set up a toy project

.
├── .git
│ <git things>
├── .vuepress
│   └── config.js
├── 1
│   └── 2
│       └── 3
│           └── 4
│               └── 5
│                   └── README.md
└── README.md

config.js

module.exports = {
  themeConfig: {
    sidebar: [
      '/',
      {
        "title": '1',
        "children": [
          {
            title: '2',
            children: [
              {
                title: '3',
                children: [
                  {
                    title: '4',
                    children: [
                      {
                        title: '5',
                        path: '/1/2/3/4/5/'
                      }
                    ]
                  }
                ]
              }
            ]
          }
        ]
      }
    ]
  }
}

Get a bunch of errors in my console:

16:30:37.368 [vuepress] detected a too deep nested sidebar group. index.js:226

Here's a very tiny gif of it working:

@KieranHunt
Copy link
Contributor Author

Will hunt around and try find the bits of the docs that refer to 3.

@haoranpb
Copy link
Contributor

The warning about the nested sidebar group is at the bottom of sidebar-groups section.

TIP

Nested sidebar group beta is also supported, but the nesting depth should be less than 3, otherwise the console will receive a warning.

@KieranHunt
Copy link
Contributor Author

Thanks @ludanxer!

Just checking in to say I haven't forgotten about this PR. Have just been super busy.

@meteorlxy
Copy link
Member

I'm going to merge this PR first, and remove the warning in docs in another commit

@meteorlxy meteorlxy changed the title fix: Remove unneeded logging for nested sidebar groups fix($theme-default): remove error logs for nested sidebar groups Mar 18, 2020
@meteorlxy meteorlxy merged commit c3a943c into vuejs:master Mar 18, 2020
@KieranHunt KieranHunt deleted the patch-1 branch March 18, 2020 09:42
larionov pushed a commit to larionov/vuepress that referenced this pull request Aug 19, 2020
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.

4 participants