-
-
Notifications
You must be signed in to change notification settings - Fork 161
docs: fix code tabs on getting started and icons pages #2805
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe PR refactors docs to replace Vue-based tabbed code components with markdown Changes
Sequence Diagram(s)(omitted — changes are documentation formatting and CSS only) Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/docs/src/docs.md (1)
52-52
: Remove stray empty<p>
elementThis lone
<p></p>
only adds an extra blank line and pollutes the HTML output.-<p></p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/docs/.vitepress/theme/Layout.vue
(1 hunks)apps/docs/src/docs.md
(5 hunks)apps/docs/src/docs/icons.md
(2 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:425-430
Timestamp: 2025-05-28T07:32:45.658Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the activeIndex synchronization within sortTabs() is necessary and correct. When sorting tabs by DOM position, indices change, so activeIndex must be updated immediately to maintain consistency with activeId. This is not an unwanted side effect but a required consequence of the sorting operation.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:240-261
Timestamp: 2025-05-28T07:01:55.095Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the complex initialization logic with updateInitialActiveIndex and updateInitialActiveId flags is necessary for SSR compatibility. Tab initialization cannot be deferred to mounted lifecycle because tabs must be available for server-side rendering of the initial HTML state.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:384-404
Timestamp: 2025-05-28T07:57:19.915Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the activeIndex and activeId watchers are intentionally designed with separation of concerns: activeIndex watcher handles activate-tab event emission and complex validation logic, while activeId watcher is kept simple for synchronization between activeId and activeIndex values only.
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2701
File: apps/docs/src/docs/migration-guide.md:622-626
Timestamp: 2025-06-05T11:43:10.793Z
Learning: In migration guides, links to the old/previous version's documentation (like bootstrap-vue.org) are appropriate and helpful when explaining deprecated features, as they provide users with reference points for what they're migrating from.
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type parameter for table items should use the singular form `Item` rather than the plural `Items` to improve readability and follow TypeScript conventions. This change would primarily affect two files: `packages/bootstrap-vue-next/src/types/ComponentProps.ts` and `apps/docs/src/data/components/table.data.ts`.
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2689
File: apps/docs/src/docs/components/demo/ProgressCustomLabels.vue:6-8
Timestamp: 2025-05-09T17:28:39.323Z
Learning: In Vue templates, when using Prettier for formatting, tags may be split across multiple lines with patterns like `<span\n>Content</span\n>` where closing angle brackets appear at the beginning of lines. This is valid syntax even though it looks unusual, and should be maintained when Prettier enforces it.
📚 Learning: 2025-05-09T17:28:39.323Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2689
File: apps/docs/src/docs/components/demo/ProgressCustomLabels.vue:6-8
Timestamp: 2025-05-09T17:28:39.323Z
Learning: In Vue templates, when using Prettier for formatting, tags may be split across multiple lines with patterns like `<span\n>Content</span\n>` where closing angle brackets appear at the beginning of lines. This is valid syntax even though it looks unusual, and should be maintained when Prettier enforces it.
Applied to files:
apps/docs/.vitepress/theme/Layout.vue
apps/docs/src/docs.md
📚 Learning: 2025-06-05T11:43:10.793Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2701
File: apps/docs/src/docs/migration-guide.md:622-626
Timestamp: 2025-06-05T11:43:10.793Z
Learning: In migration guides, links to the old/previous version's documentation (like bootstrap-vue.org) are appropriate and helpful when explaining deprecated features, as they provide users with reference points for what they're migrating from.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-05-23T23:58:07.165Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2701
File: apps/docs/src/docs/migration-guide.md:630-632
Timestamp: 2025-05-23T23:58:07.165Z
Learning: The `<NotYetImplemented/>` component in the bootstrap-vue-next documentation automatically renders text indicating "Not Yet Implemented", so additional explanatory text about features not being implemented is redundant when this component is used.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-05-28T07:01:55.095Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:240-261
Timestamp: 2025-05-28T07:01:55.095Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the complex initialization logic with updateInitialActiveIndex and updateInitialActiveId flags is necessary for SSR compatibility. Tab initialization cannot be deferred to mounted lifecycle because tabs must be available for server-side rendering of the initial HTML state.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-05-28T07:57:19.915Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:384-404
Timestamp: 2025-05-28T07:57:19.915Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the activeIndex and activeId watchers are intentionally designed with separation of concerns: activeIndex watcher handles activate-tab event emission and complex validation logic, while activeId watcher is kept simple for synchronization between activeId and activeIndex values only.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-06-26T19:46:19.333Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2762
File: apps/docs/src/docs/components/tooltip.md:0-0
Timestamp: 2025-06-26T19:46:19.333Z
Learning: BTooltip is a very thin wrapper around BPopover in bootstrap-vue-next. There is no separate `useTooltipController` composable - the `usePopoverController` composable can be used to programmatically control both popovers and tooltips.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-05-28T07:32:45.658Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2716
File: packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue:425-430
Timestamp: 2025-05-28T07:32:45.658Z
Learning: In BTabs component (packages/bootstrap-vue-next/src/components/BTabs/BTabs.vue), the activeIndex synchronization within sortTabs() is necessary and correct. When sorting tabs by DOM position, indices change, so activeIndex must be updated immediately to maintain consistency with activeId. This is not an unwanted side effect but a required consequence of the sorting operation.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-05-26T17:28:35.902Z
Learnt from: VividLemon
PR: bootstrap-vue-next/bootstrap-vue-next#2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:85-86
Timestamp: 2025-05-26T17:28:35.902Z
Learning: In the `useBLinkTagResolver` function in `packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts`, the `routerComponentName` parameter is required and typed as `MaybeRefOrGetter<string>`, ensuring it cannot be undefined.
Applied to files:
apps/docs/src/docs/icons.md
apps/docs/src/docs.md
📚 Learning: 2025-04-24T20:35:48.629Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type parameter for table items should use the singular form `Item` rather than the plural `Items` to improve readability and follow TypeScript conventions. This change would primarily affect two files: `packages/bootstrap-vue-next/src/types/ComponentProps.ts` and `apps/docs/src/data/components/table.data.ts`.
Applied to files:
apps/docs/src/docs.md
📚 Learning: 2025-04-27T17:30:42.045Z
Learnt from: unstoppablecarl
PR: bootstrap-vue-next/bootstrap-vue-next#2671
File: packages/bootstrap-vue-next/src/components/BToast/_toast.scss:1-3
Timestamp: 2025-04-27T17:30:42.045Z
Learning: In global SCSS files for bootstrap-vue-next, `:deep()` combinator should not be used as it only works in Vue's scoped style blocks. Standard CSS/SCSS selectors should be used instead.
Applied to files:
apps/docs/src/docs.md
📚 Learning: 2025-04-28T22:48:46.738Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2425
File: packages/bootstrap-vue-next/src/plugins/popoverController/index.ts:138-140
Timestamp: 2025-04-28T22:48:46.738Z
Learning: In bootstrap-vue-next, `PopoverOrchestratorParam` includes an `id` field of type `ControllerKey` through inheritance from base types. This field is declared in the `BvControllerOptions` interface and propagated through the type hierarchy.
Applied to files:
apps/docs/src/docs.md
📚 Learning: 2025-05-01T23:40:56.146Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2679
File: apps/docs/.vitepress/theme/Layout.vue:0-0
Timestamp: 2025-05-01T23:40:56.146Z
Learning: In bootstrap-vue-next, the `useScrollspy` hook returns an object with a `current` property which is the ID string (or null) of the currently active item, not an object containing an ID.
Applied to files:
apps/docs/src/docs.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
apps/docs/src/docs.md (1)
32-50
: Switch to VitePresscode-group
looks goodAll converted blocks follow the documented
::: code-group
syntax and the labelled fences (```bash [PNPM]
etc.) render correctly in VitePress ≥1.0.0-beta. Properly closes with:::
and therefore fixes the JS-driven tab crashes reported in #2803.Also applies to: 78-96, 148-166, 228-248
apps/docs/src/docs/icons.md (1)
23-41
: Icon docs refactor LGTMThe two new
code-group
blocks are syntactically valid and consistent with the rest of the documentation updates.Also applies to: 110-128
Describe the PR
I'm not entirely sure what was going on with the existing code - in both pages where we had different flavors of docs, the first one worked and the remaining ones failed. Clicking on the tabs also caused the browser to hang. In any case, for now, I'm moving to using the VitePress method of handling multiple examples in tabs and tweaking the styling a bit to fit in with the rest of the page.
I realize this loses the persistent language selection that we had previously - this is a known issue against vitepress -vuejs/vitepress#2954 - we can grab that if it gets fixed. I'll consider writing a plugin to solve this ala https://github.com/Red-Asuka/vitepress-plugin-tabs/tree/main if I have time. But for now, having a robust solutions to the core problem seems higher priority.
closes #2803
Small replication
See bug
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)
feat(...)
fix(...)
docs(...)
The PR fulfills these requirements:
CHANGELOG
is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be deniedSummary by CodeRabbit
Documentation
Style