vitepress icon indicating copy to clipboard operation
vitepress copied to clipboard

sidebar uses native <details> for collapsible groups [#3804. also #3517]

Open olets opened this issue 1 year ago • 9 comments

Had time to hack on this now, so didn't wait to hear if it was desirable. Close if you need to 👍

Closes #3804

  • #3804

Fixes #3517, closes # 3802 as obviated

  • #3517
  • ~ #3802 ~

~Fixes #3805~

  • ~#3805~
  • Reverted per https://github.com/vuejs/vitepress/pull/3806#issuecomment-2079941485

3804 is the primary target of this PR. Preserving the behaviors identified in 3517 and 3805 would have taken additional work, so I took the opportunity to address them for "free".

Before

Sidebar collapsible groups used custom markup and JS. Carets are focusable, an accessibility problem.

After

  • 3804: Sidebar collapsible groups use native <details> elements. Custom markup and JS is reduced.
  • 3517: dropped the separately-clickable carets. See PR #3802 for details on rationale.
  • 3805: Clicking linked text in a collapsible group's heading doesn't toggle the group.

To test

  1. With the released version, replace docs/.vitepress/config/en.ts's sidebarGuide with the following.

    function sidebarGuide(): DefaultTheme.SidebarItem[] {
      return [
        {
          text: 'Introduction',
          collapsed: true,
          items: [
            {
              text: 'Group heading linked to "What is VitePress?"',
              link: 'what-is-vitepress',
              collapsed: true,
              items: [
                {
                  text: 'Another group heading linked to "What is VitePress?"',
                  link: 'what-is-vitepress',
                  collapsed: true,
                  items: [
                    {
                      text: 'What is VitePress?',
                      link: 'what-is-vitepress',
                    },
                  ],
                },
              ],
            },
            { text: 'Getting Started', link: 'getting-started' },
            { text: 'Routing', link: 'routing' },
            { text: 'Deploy' }
          ]
        },
        { text: 'What is VitePress?', link: 'what-is-vitepress',},
        {
          text: 'Writing',
          collapsed: false,
          items: [
            { text: 'Markdown Extensions', link: 'markdown' },
            { text: 'Asset Handling', link: 'asset-handling' },
            { text: 'Frontmatter', link: 'frontmatter' },
            { text: 'Using Vue in Markdown', link: 'using-vue' },
            { text: 'Internationalization', link: 'i18n' }
          ]
        },
        {
          text: 'Customization',
          collapsed: false,
          items: [
            { text: 'Using a Custom Theme', link: 'custom-theme' },
            {
              text: 'Extending the Default Theme',
              link: 'extending-default-theme'
            },
            { text: 'Build-Time Data Loading', link: 'data-loading' },
            { text: 'SSR Compatibility', link: 'ssr-compat' },
            { text: 'Connecting to a CMS', link: 'cms' }
          ]
        },
        {
          text: 'Experimental',
          collapsed: false,
          items: [
            { text: 'MPA Mode', link: 'mpa-mode' },
            { text: 'Sitemap Generation', link: 'sitemap-generation' }
          ]
        },
        { text: 'Config & API Reference', base: '/reference/', link: 'site-config' }
      ]
    }
    
  2. Run pnpm run docs

  3. Open http://localhost:<your port>/guide/what-is-vitepress in a browser

  4. Confirm that the "Introduction" group is closed. [3804 regression check]

  5. Toggle "Introduction" and its children open by clicking on the carets [3517]. Confirm that all items linked to "What is VitePress?" have the active color. [3804 regression check]

  6. Toggle unlinked group headings, eg. "Writing". Confirm that they toggle when either the text or caret is clicked. [3517]

  7. Confirm that the non-group item linked to "What is VitePress?" has the indicator. [3804 regression check]

  8. Toggle "Introduction" > "Group heading linked to “What is VitePress?”" closed.

  9. Confirm that hovering over "Introduction" > "Deploy", which is not linked, does not give it the hover color. [3804 regression check]

  10. Confirm that hovering of the text "Group heading linked to “What is VitePress?”" gives it the hover color. [3804]

  11. Confirm that hovering with the cursor over linked sidebar items gives them the hover color. [3804 regression check]

  12. Click "Introduction" > "Routing"

  13. ~Under "Introduction", click the text of the first item, "Group heading linked to "“What is VitePress?”". Confirm that the site navigates and the group stays closed. [3805]~ Reverted per https://github.com/vuejs/vitepress/pull/3806#issuecomment-2079941485

olets avatar Apr 18 '24 06:04 olets

(sorry for that noise. almost never resolve merge conflicts via the github UI and forgot it would merge the base into the feature. disregard the "'main' into sidebar-details", it's reverted)

olets avatar Apr 18 '24 18:04 olets

Also, regarding #3805 I don't think that's a bug. If I am on this state:

image

I would want clicking next page will uncollapse the "Writing" section in sidebar.

brc-dd avatar Apr 26 '24 18:04 brc-dd

Also, the tests seem to be failing, please update those too.

brc-dd avatar Apr 26 '24 18:04 brc-dd

Thanks for the feedback. Been meaning to say that I see the failing tests and will look at them. Will try to find time for this soon.

olets avatar Apr 26 '24 20:04 olets

(ignore that test rerun — I only rebased, should still fail)

I have the updates locally. Waiting on https://github.com/vuejs/core/pull/10938, then will push them up.

@brc-dd do you have a preference for whether I make new commits or rewrite commits? If new commits, chore type to keep "feat: update the new feature" noise out of the changelog?

olets avatar May 16 '24 01:05 olets

No preference. You can just keep pushing things. No need to write in conventional commit format too. We do squash and merge, so it's the PR title that matters. Commit messages of individual commits will be ignored. You don't need to maintain linear history either, feel free to just merge things instead of rebase or force-pushes.

brc-dd avatar May 16 '24 03:05 brc-dd

Ok pushed up the updates I referred to.

Sorry again for the noise. If I'd noticed the broken test before opening the PR, I would have made it a draft or held off.

Still blocked. Waiting for ~https://github.com/vuejs/core/commit/fd18ce70b1a260a2485c9cd7faa30193da4b79f5 to be released~ edit: done, and then for ~Vitepress's Vue dependency to be upgraded~ edit: done.

The type error doesn't impact the latest Netlify preview. Ready for browser QA now or later.

olets avatar May 27 '24 22:05 olets

Ready for you @brc-dd

olets avatar Jun 25 '24 01:06 olets