feat(BSort)!: Implement initial-sort-direction and move compare from sort-by to fields #2777
Conversation
|
|
WalkthroughAdds table-level props Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BTable
participant Comparer
participant SortIcon
User->>BTable: Click column header
BTable->>BTable: initialDir = field.initialSortDirection ?? props.initialSortDirection
BTable->>BTable: resolveOrder (consider 'last', last-used, mustSort)
alt order set
BTable->>Comparer: comparer = field.sortCompare || props.sortCompare || default
BTable->>BTable: sort items by key using comparer
BTable-->>User: emit 'sorted' and update rows
BTable->>SortIcon: render active state (opacity = 1)
else order cleared
BTable->>BTable: remove/clear sort entry
BTable-->>User: emit 'sorted' (cleared)
BTable->>SortIcon: render unsorted state (opacity < 1)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/docs/src/docs/components/demo/TableInitialSortDirection.vue (1)
2-6: Consider adding table-level initialSortDirection to better demonstrate the feature.The demo component effectively shows field-level
initialSortDirection(line 31), but consider adding the table-levelinitialSortDirectionprop to theBTablecomponent to fully demonstrate both aspects of this new feature.<BTable + initial-sort-direction="desc" :sort-by="[{key: 'first_name', order: 'desc'}]" :items="items" :fields="fields" />This would showcase how the table-level prop serves as a default for fields that don't specify their own
initialSortDirection.apps/docs/src/docs/components/table.md (2)
111-111: Wrong intra-doc link fragment
#custom-sort-comparer-sdoes not match GitHub-style anchors (should be#custom-sort-comparers). Broken links hurt navigation and MLint (MD051) reports this.-See [Custom Sort Comparer(s)](#custom-sort-comparer-s) for more details. +See [Custom Sort Comparer(s)](#custom-sort-comparers) for more details.
748-748: Typo: “precendence” → “precedence”-The field-level prop takes precendence. +The field-level prop takes precedence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/docs/src/data/components/table.data.ts(1 hunks)apps/docs/src/docs/components/demo/TableComplete.vue(5 hunks)apps/docs/src/docs/components/demo/TableInitialSortDirection.vue(1 hunks)apps/docs/src/docs/components/demo/TableProvider.ts(1 hunks)apps/docs/src/docs/components/demo/TableProvider.vue(1 hunks)apps/docs/src/docs/components/demo/TableProviderAsync.vue(1 hunks)apps/docs/src/docs/components/demo/TableSortBy.ts(0 hunks)apps/docs/src/docs/components/demo/TableSortByCustom.vue(2 hunks)apps/docs/src/docs/components/demo/TableSortCompareCustom.ts(1 hunks)apps/docs/src/docs/components/table.md(4 hunks)apps/docs/src/docs/migration-guide.md(1 hunks)apps/playground/src/components/Comps/TTable.vue(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(5 hunks)packages/bootstrap-vue-next/src/components/BTable/table.spec.ts(1 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(2 hunks)packages/bootstrap-vue-next/src/types/TableTypes.ts(2 hunks)packages/bootstrap-vue-next/src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/src/docs/components/demo/TableSortBy.ts
🧰 Additional context used
🧠 Learnings (17)
📓 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: 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`.
apps/docs/src/docs/components/demo/TableProvider.vue (4)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
packages/bootstrap-vue-next/src/types/index.ts (4)
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: 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: 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.
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.
apps/docs/src/docs/components/demo/TableSortCompareCustom.ts (4)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
apps/docs/src/docs/components/demo/TableProviderAsync.vue (2)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
apps/docs/src/docs/components/demo/TableProvider.ts (8)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
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#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.
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.
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.
apps/docs/src/docs/components/demo/TableInitialSortDirection.vue (5)
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: 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: 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: 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.
apps/docs/src/docs/migration-guide.md (4)
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: 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: 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.
packages/bootstrap-vue-next/src/types/ComponentProps.ts (7)
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: 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#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.
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: 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.
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.
apps/playground/src/components/Comps/TTable.vue (5)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
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.
apps/docs/src/docs/components/demo/TableSortByCustom.vue (6)
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: 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: VividLemon
PR: bootstrap-vue-next/bootstrap-vue-next#2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:82-83
Timestamp: 2025-05-26T17:27:44.251Z
Learning: In Vue 3.3+, `toRef()` can accept any input type including raw values, existing refs, and getter functions (MaybeRefOrGetter). It will create a ref from the input or use the existing ref if one is passed. Using `toRef()` for simple property access is more performant than `computed()` because it avoids the caching overhead that computeds provide.
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.
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (4)
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: 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: 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#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.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (8)
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: 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: 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#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.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
packages/bootstrap-vue-next/src/types/TableTypes.ts (6)
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: 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#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.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
apps/docs/src/docs/components/demo/TableComplete.vue (4)
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: 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: 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.
apps/docs/src/data/components/table.data.ts (3)
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: 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: 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.
apps/docs/src/docs/components/table.md (3)
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#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.
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.
🧬 Code Graph Analysis (2)
apps/docs/src/docs/components/demo/TableProvider.ts (1)
packages/bootstrap-vue-next/src/types/TableTypes.ts (1)
BTableSortBy(32-35)
packages/bootstrap-vue-next/src/types/ComponentProps.ts (2)
packages/bootstrap-vue-next/src/types/index.ts (2)
BTableSortBy(21-21)BTableInitialSortDirection(18-18)packages/bootstrap-vue-next/src/types/TableTypes.ts (2)
BTableSortBy(32-35)BTableInitialSortDirection(27-27)
🪛 markdownlint-cli2 (0.17.2)
apps/docs/src/docs/components/table.md
108-108: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
111-111: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (26)
packages/bootstrap-vue-next/src/types/index.ts (1)
18-18: LGTM! Clean type export addition.The new
BTableInitialSortDirectiontype is properly exported and maintains consistency with the existing TableTypes export structure.apps/docs/src/docs/components/demo/TableProvider.ts (1)
5-5: LGTM! Type simplification aligns with sorting API refactor.The removal of the generic parameter from
BTableSortByis consistent with the broader refactoring to simplify the sorting API and matches the updated type definition.apps/docs/src/docs/components/demo/TableProvider.vue (1)
107-107: LGTM! Consistent type usage with simplified API.The removal of the generic parameter from
BTableProviderContextmaintains consistency with the type simplification and doesn't affect the function logic.apps/docs/src/docs/migration-guide.md (1)
718-723: LGTM! Accurate migration documentation.The migration guide updates correctly reflect the API changes:
- Clear deprecation notice for
sort-comparewith proper replacement guidance- Accurate renaming of
sort-directiontoinitial-sort-direction- Consistent with the PR objectives and implementation changes
apps/docs/src/docs/components/demo/TableSortCompareCustom.ts (1)
7-12: LGTM! Clean demonstration of the new field-level comparer API.The refactoring effectively demonstrates the new approach:
- Moving from sortBy array to field definition object
- Explicit
sortable: trueflag- Comparer function properly defined at field level
- Preserved comparison logic for removing articles
This aligns well with the broader API changes moving compare functions from sort-by to field definitions.
apps/docs/src/docs/components/demo/TableProviderAsync.vue (1)
107-107: LGTM: Type simplification aligns with codebase changes.The removal of the generic parameter from
BTableProviderContext<Person>toBTableProviderContextis consistent with the broader type simplification efforts mentioned in the PR objectives. This change maintains functionality while reducing type complexity.apps/docs/src/data/components/table.data.ts (1)
588-593: LGTM: Well-documented prop addition.The new
initialSortDirectionprop is properly documented with:
- Appropriate type
BTableInitialSortDirection- Sensible default value
'asc'for backward compatibility- Clear description explaining the feature and its precedence rules
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
2-2: LGTM: Unused import removal aligns with test case removal.The removal of the
viimport is appropriate as it was only used in the deleted test case "sorting does not wipe out the comparer function." This change aligns with the API shift where comparer functions moved from thesortBymodel to field definitions.packages/bootstrap-vue-next/src/types/ComponentProps.ts (2)
60-60: LGTM: Required import for new prop type.The addition of
BTableInitialSortDirectionto the imports is necessary for the newinitialSortDirectionprop in theBTablePropsinterface.
128-130: LGTM: Type updates align with API changes.The changes correctly reflect the new sorting API:
- Removal of generic parameter from
BTableSortBy<Items>[]toBTableSortBy[]aligns with type simplification- Addition of optional
initialSortDirectionprop with proper type annotationapps/docs/src/docs/components/demo/TableInitialSortDirection.vue (1)
28-33: LGTM: Good demonstration of field-level initialSortDirection.The fields array effectively demonstrates the new field-level
initialSortDirectionproperty, showing how theagefield can have a different initial sort direction ('desc') from the default.apps/playground/src/components/Comps/TTable.vue (1)
552-560: LGTM! Type simplification aligns with the updated API.The removal of generic type parameters from
BTableSortByin bothonSortedandonSortUpdatefunctions is consistent with the broader refactoring across the codebase.apps/docs/src/docs/components/demo/TableSortByCustom.vue (3)
18-18: Correct import update for the new API.The change from
TableFieldRawtoTableFieldis appropriate as the field definitions now include thecomparerproperty.
51-56: Properly migrated comparer to field definitions.The comparer function has been correctly moved from the sortBy model to the field definitions, aligning with the new API design where custom comparers are field-level properties.
58-61: Correctly updated sortBy structure.The removal of the
comparerproperty from the sortBy entries is consistent with the API changes.apps/docs/src/docs/components/demo/TableComplete.vue (4)
153-175: Well-implemented UI for initial sort direction control.The form group provides a clean interface for users to select the initial sort direction, with clear labels for each option.
191-191: Correctly integrated the new prop.The
initial-sort-directionprop is properly bound to the BTable component.
251-251: Nice enhancement to display active filters.Showing the active filter fields improves user awareness of the current filtering state.
345-355: Clean implementation of sort direction options.The options array provides user-friendly labels, and the reactive variable is properly typed with the imported
BTableInitialSortDirectiontype.packages/bootstrap-vue-next/src/types/TableTypes.ts (3)
24-28: Well-documented new type for initial sort direction.The
BTableInitialSortDirectiontype is clearly defined with helpful documentation explaining the 'last' option functionality.
32-35: Clean simplification of BTableSortBy type.Removing the generic parameter and
comparerproperty simplifies the API and moves the complexity to where it belongs - the field definitions.
57-72: Comprehensive enhancement of TableField type.The addition of
comparer,initialSortDirection, and other properties provides a rich set of field-level configuration options. The property ordering improves readability.packages/bootstrap-vue-next/src/components/BTable/BTable.vue (4)
256-256: Appropriate default value for initial sort direction.Setting 'asc' as the default maintains backward compatibility while enabling the new feature.
719-722: Correctly integrated field-level comparers.The sorting logic now properly retrieves and uses the comparer function from the field definition during multi-sort operations.
820-843: Well-structured logic for determining initial sort direction.The implementation follows a clear precedence hierarchy:
- Field-level
initialSortDirection(if specified)- Table-level
initialSortDirectionprop- Default to 'asc'
The special handling of the 'last' option is particularly useful for maintaining user context.
847-855: Correct integration with mustSort logic.The
resolveOrderfunction properly applies the initial sort direction whenmustSortis active or when toggling from an undefined state.
|
The field definitions in 
GH is not allowing me to comment on those lines. |
Yeah, not sure why typescript isn't catching that. I've factored out a separate example with the field-level |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/docs/src/docs/types.md (2)
501-501: Clarify what'last'represents in the new union
'asc'/'desc'are self-explanatory, but'last'is ambiguous to readers skimming the docs. A brief inline comment or a one-sentence description in the surrounding markdown would remove any guesswork about whether it means “keep previously-applied order”, “unsorted”, or something else.
505-519: Add concrete examples for the newly-addedTableFieldpropsNice addition of
filterByFormatted,initialSortDirection,isRowHeader,stickyColumn,tdAttr, andthAttr. Because these accept either primitives or callback signatures, quick code snippets (e.g. “tdAttr: (value) => ({title: value})”) would make the intent crystal-clear for consumers scanning the docs.No code changes required—purely a documentation enhancement.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
582-584: Track accessibility and styling improvementsThe TODO correctly identifies missing
aria-selectedattributes for row selection accessibility. This should be tracked as a follow-up issue to ensure proper ARIA support.Would you like me to create an issue to track the implementation of
aria-selectedattributes and the functionaltbodyTrAttrssupport?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/src/docs/components/demo/TableComplete.vue(7 hunks)apps/docs/src/docs/components/table.md(5 hunks)apps/docs/src/docs/types.md(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/docs/src/docs/components/table.md
- apps/docs/src/docs/components/demo/TableComplete.vue
🧰 Additional context used
🧠 Learnings (3)
📓 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: 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`.
apps/docs/src/docs/types.md (1)
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`.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (8)
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: 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: 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#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.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
🔇 Additional comments (9)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (9)
2-16: Clean implementation of reusable SVG iconsGood use of
createReusableTemplateto avoid duplicating SVG markup. The dynamic binding of icon properties and accessibility attributes is well implemented.
191-191: Necessary import additionsGood catch on adding the missing
useToNumberimport that was already being used in the code, along with the newcreateReusableTemplatefor the icon implementation.
198-198: Required type import for new functionalityThe
BTableInitialSortDirectiontype import is correctly added to support the new initial sort direction feature.
233-233: Appropriate default value for new propSetting
'asc'as the default initial sort direction is a sensible choice that maintains backward compatibility.
315-315: Type signature update aligned with API changesThe removal of the generic parameter from
BTableSortByis consistent with the architectural change of moving the comparer function to field definitions.
695-700: Correct implementation of comparer relocationThe refactoring properly moves the comparer function from the sort model to field definitions, improving the separation of concerns and aligning with the PR objectives.
796-820: Well-structured sort direction logicThe helper functions correctly implement the initial sort direction feature with proper precedence (field-level > table-level) and the special 'last' value handling.
821-841: Proper integration of initial sort directionThe sort handling correctly uses the new
getInitialSortDirection()helper when creating new sort entries, and appropriately handles the mustSort logic with the initial direction.
1025-1065: Excellent implementation of dynamic sort iconsThe icon logic provides clear visual feedback and correctly reflects the initial sort direction in the unsorted state. The handling of the 'last' special case is consistent with the sorting behavior.
…e compare function
…e-level sort-comparer
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
937-1091: Comprehensive test coverage for initial sort direction feature.This test suite excellently covers all aspects of the
initialSortDirectionfunctionality:
- ✅ Table-level
initialSortDirectionprop ('asc', 'desc')- ✅ Field-level override behavior
- ✅ Special 'last' direction value handling
- ✅ Fallback behavior when no previous sort exists
- ✅ UI visual feedback (sort icon opacity)
- ✅ Proper async user interaction testing
The tests are well-structured, cover edge cases, and validate both functional behavior and visual indicators. The special handling of the 'last' direction value is particularly well-tested.
Consider making the icon opacity test more specific by checking for exact SVG elements or classes rather than just the presence of opacity styles, but the current implementation is adequate for the testing needs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/docs/src/data/components/table.data.ts(2 hunks)apps/docs/src/docs/components/demo/TableComplete.vue(7 hunks)apps/docs/src/docs/components/demo/TableProvider.vue(1 hunks)apps/docs/src/docs/components/demo/TableProviderAsync.vue(1 hunks)apps/docs/src/docs/components/demo/TableRowStyles.vue(1 hunks)apps/docs/src/docs/components/demo/TableSortByCustom.vue(2 hunks)apps/docs/src/docs/components/demo/TableSortCompareCustom.ts(1 hunks)apps/docs/src/docs/components/table.md(5 hunks)apps/docs/src/docs/migration-guide.md(1 hunks)apps/docs/src/docs/types.md(1 hunks)apps/playground/src/components/Comps/TTable.vue(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(13 hunks)packages/bootstrap-vue-next/src/components/BTable/table.spec.ts(2 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(2 hunks)packages/bootstrap-vue-next/src/types/TableTypes.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/docs/src/docs/components/demo/TableSortCompareCustom.ts
- apps/playground/src/components/Comps/TTable.vue
- apps/docs/src/docs/components/demo/TableProvider.vue
- apps/docs/src/docs/components/demo/TableProviderAsync.vue
- apps/docs/src/docs/components/demo/TableSortByCustom.vue
- apps/docs/src/data/components/table.data.ts
- packages/bootstrap-vue-next/src/types/ComponentProps.ts
- apps/docs/src/docs/types.md
- apps/docs/src/docs/components/demo/TableComplete.vue
- apps/docs/src/docs/components/table.md
- packages/bootstrap-vue-next/src/components/BTable/BTable.vue
- packages/bootstrap-vue-next/src/types/TableTypes.ts
🧰 Additional context used
🧠 Learnings (4)
📓 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: 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`.
apps/docs/src/docs/components/demo/TableRowStyles.vue (4)
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#2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability. This makes expressions like `item: Item` and `items: Item[]` more intuitive than `item: Items` and `items: Items[]`.
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 for table items should be named in singular form (`Item`) rather than plural (`Items`) to follow TypeScript conventions and improve readability in type definitions.
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 for table items should use the singular form `Item` rather than the plural `Items` to follow TypeScript conventions and improve readability in type definitions.
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (4)
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: 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: 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#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.
apps/docs/src/docs/migration-guide.md (3)
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: 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: 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`.
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
packages/bootstrap-vue-next/src/types/TableTypes.ts (2)
TableItem(9-13)TableField(56-82)
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[grammar] ~718-~718: Ensure spelling is correct
Context: ...internal consistency. Sorting has been significatly reworked read the [sorting section](/docs/compon...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~723-~723: Ensure spelling is correct
Context: ...d, if you need to customize sorting for locaization, the documentation on [custom sort comp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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 (4)
apps/docs/src/docs/components/demo/TableRowStyles.vue (1)
28-29: Excellent type safety improvement!The change from
PersontoPerson | nullwith optional chaining is a solid defensive programming practice that prevents potential runtime errors when the table component passes null items to the row class function.packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (3)
2-2: LGTM: Import cleanup aligns with test restructuring.The removal of the
viimport is consistent with the replacement of the previous single test with the comprehensive new test suites.
755-854: Excellent comprehensive test coverage for custom sort comparers.This test suite effectively covers all aspects of the custom sort comparer functionality:
- ✅ Table-level
sortCompareprop usage- ✅ Field-level
sortCompareoverride behavior- ✅ Fallback to default comparison
- ✅ Type-safe comparer function implementations
- ✅ Realistic test scenarios with clear assertions
The tests follow best practices with descriptive names, proper TypeScript typing, and comprehensive coverage of the feature's behavior.
856-935: Strong integration testing for combined sorting features.These tests effectively validate that the new sorting features work correctly when used together:
- ✅ Custom comparer combined with custom initial sort direction
- ✅ Multi-sort mode with mixed comparers (default + custom)
- ✅ Proper field-level configuration testing
- ✅ Realistic user interaction simulation
The integration testing approach ensures the features are robust when used in combination, which is critical for a complex feature like table sorting.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/docs/src/docs/migration-guide.md (2)
718-718: Add missing punctuation for readabilityInsert a semicolon (or period) before “read” to separate the two independent clauses.
-Sorting has been significantly reworked read the [sorting section](/docs/components/table#sorting) of our documentation. Some specific changes include the following: +Sorting has been significantly reworked; read the [sorting section](/docs/components/table#sorting) of our documentation for full details. Some specific changes include the following:
723-723: Fix typo and split the run-on sentence“localizatio” → “localization”, and break the sentence for clarity.
- The internal `sort-compare` routine has been simplified, if you need to customize sorting for localizatio, the documentation on [custom sort comparers](/docs/components/table#custom-sort-comparers) for details. + The internal `sort-compare` routine has been simplified. If you need to customize sorting for localization, see the documentation on [custom sort comparers](/docs/components/table#custom-sort-comparers) for details.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/src/docs/migration-guide.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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: 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`.
apps/docs/src/docs/migration-guide.md (3)
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: 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: 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`.
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[grammar] ~723-~723: Ensure spelling is correct
Context: ...d, if you need to customize sorting for localizatio, the documentation on [custom sort comp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
1020-1104: LGTM! Well-implemented SortIcon component with minor suggestion.The
SortIconcomponent is well-designed with proper handling of all sort states and good accessibility practices. The render function approach is appropriate for this simple SVG component.Consider extracting the direction resolution logic to reduce duplication between
iconPathandiconClass:setup(props) { const currentSort = computed(() => props.sortBy?.find((el) => el.key === props.field.key)) + const resolvedDirection = computed(() => { + if (currentSort.value?.order) return currentSort.value.order + + const fieldInitialDirection = props.field.initialSortDirection || props.initialSortDirection + + if (fieldInitialDirection === 'last') { + const lastSorted = props.sortBy?.find((sort) => sort.order !== undefined) + return lastSorted?.order || 'asc' + } + + return fieldInitialDirection === 'desc' ? 'desc' : 'asc' + }) const iconPath = computed(() => { - if (currentSort.value?.order === 'asc') return ascPath - if (currentSort.value?.order === 'desc') return descPath - // ... rest of logic + return resolvedDirection.value === 'desc' ? descPath : ascPath }) const iconClass = computed(() => { - if (currentSort.value?.order === 'asc') return 'bi bi-arrow-up-short' - if (currentSort.value?.order === 'desc') return 'bi bi-arrow-down-short' - // ... rest of logic + return resolvedDirection.value === 'desc' ? 'bi bi-arrow-down-short' : 'bi bi-arrow-up-short' })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)
🧠 Learnings (2)
📓 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: 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`.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (10)
Learnt from: xvaara
PR: #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: dwgray
PR: #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: xvaara
PR: #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: #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: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability. This makes expressions like item: Item and items: Item[] more intuitive than item: Items and items: Items[].
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should be named in singular form (Item) rather than plural (Items) to follow TypeScript conventions and improve readability in type definitions.
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability in type definitions.
Learnt from: VividLemon
PR: #2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:82-83
Timestamp: 2025-05-26T17:27:44.251Z
Learning: In Vue 3.3+, toRef() can accept any input type including raw values, existing refs, and getter functions (MaybeRefOrGetter). It will create a ref from the input or use the existing ref if one is passed. Using toRef() for simple property access is more performant than computed() because it avoids the caching overhead that computeds provide.
Learnt from: VividLemon
PR: #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.
Learnt from: xvaara
PR: #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.
🧰 Additional context used
🧠 Learnings (2)
📓 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: 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`.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (10)
Learnt from: xvaara
PR: #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: dwgray
PR: #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: xvaara
PR: #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: #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: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability. This makes expressions like item: Item and items: Item[] more intuitive than item: Items and items: Items[].
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should be named in singular form (Item) rather than plural (Items) to follow TypeScript conventions and improve readability in type definitions.
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability in type definitions.
Learnt from: VividLemon
PR: #2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:82-83
Timestamp: 2025-05-26T17:27:44.251Z
Learning: In Vue 3.3+, toRef() can accept any input type including raw values, existing refs, and getter functions (MaybeRefOrGetter). It will create a ref from the input or use the existing ref if one is passed. Using toRef() for simple property access is more performant than computed() because it avoids the caching overhead that computeds provide.
Learnt from: VividLemon
PR: #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.
Learnt from: xvaara
PR: #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.
⏰ 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 (9)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (9)
116-151: LGTM! Improved sort icon rendering with reusable component.The refactoring from inline SVG to a reusable
SortIconcomponent improves maintainability and consistency. The component receives appropriate props for dynamic rendering based on field configuration and sort state.
188-188: LGTM! Necessary imports for the new component.The added imports (
defineComponent,h) are required for the programmaticSortIconcomponent definition.
194-194: LGTM! Required type import.The
BTableInitialSortDirectiontype import is necessary for the newinitialSortDirectionprop.
229-229: LGTM! Sensible default for initial sort direction.The default value of
'asc'forinitialSortDirectionis logical and matches typical user expectations for table sorting.
246-246: LGTM! Appropriate default for sort compare function.Setting
sortComparetoundefinedby default allows the component to fall back to the built-in string comparison logic when no custom comparer is provided.
312-312: LGTM! Simplified event signature.Removing the generic parameter from the
sortedevent signature simplifies the API without losing functionality, as comparers are now handled at the field level.
793-816: LGTM! Well-designed helper functions for sort direction.The helper functions are clean and well-structured:
getLastSortDirection()efficiently finds the last sorted field's directiongetInitialSortDirection()implements proper precedence (field-level → table-level → default)- Special handling for the
'last'option is correctly implemented- Functions follow single responsibility principle
818-876: LGTM! Enhanced sorting logic with proper initial direction handling.The updated sorting logic correctly integrates the new initial sort direction functionality:
- Proper use of helper functions for determining initial directions
- Clean handling of multi-sort scenarios including removal of undefined orders
- Maintains backward compatibility while adding new features
- Logic is clear and well-structured
692-704: LGTM! Enhanced multi-sort with field-level comparer support.The multi-sort logic correctly implements the comparer precedence:
- Field-level
sortComparetakes priority over table-levelsortCompare- Proper fallback to default string comparison when no custom comparer is provided
- Maintains the existing multi-sort algorithm while adding flexibility
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
1020-1087: Well-implemented SortIcon component with room for optimizationThe
SortIconcomponent correctly handles:
- Complex initial sort direction logic including the
'last'option- Proper visual states (path, class, opacity) based on sort status
- Type-safe props and computed properties
However, consider extracting this component to a separate file for better performance (avoid recreation on parent re-renders) and reusability.
Consider extracting SortIcon to a separate component file:
// Create packages/bootstrap-vue-next/src/components/BTable/BSortIcon.vue // This would prevent component recreation on every BTable re-render
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)
🧠 Learnings (2)
📓 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: 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`.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (10)
Learnt from: xvaara
PR: #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: dwgray
PR: #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: xvaara
PR: #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: #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.
Learnt from: xvaara
PR: #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: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability. This makes expressions like item: Item and items: Item[] more intuitive than item: Items and items: Items[].
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should be named in singular form (Item) rather than plural (Items) to follow TypeScript conventions and improve readability in type definitions.
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability in type definitions.
Learnt from: VividLemon
PR: #2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:82-83
Timestamp: 2025-05-26T17:27:44.251Z
Learning: In Vue 3.3+, toRef() can accept any input type including raw values, existing refs, and getter functions (MaybeRefOrGetter). It will create a ref from the input or use the existing ref if one is passed. Using toRef() for simple property access is more performant than computed() because it avoids the caching overhead that computeds provide.
Learnt from: VividLemon
PR: #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.
🧰 Additional context used
🧠 Learnings (2)
📓 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: 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`.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (10)
Learnt from: xvaara
PR: #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: dwgray
PR: #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: xvaara
PR: #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: #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.
Learnt from: xvaara
PR: #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: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability. This makes expressions like item: Item and items: Item[] more intuitive than item: Items and items: Items[].
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should be named in singular form (Item) rather than plural (Items) to follow TypeScript conventions and improve readability in type definitions.
Learnt from: dwgray
PR: #2669
File: apps/docs/src/data/components/table.data.ts:334-336
Timestamp: 2025-04-24T20:35:48.629Z
Learning: The generic type for table items should use the singular form Item rather than the plural Items to follow TypeScript conventions and improve readability in type definitions.
Learnt from: VividLemon
PR: #2691
File: packages/bootstrap-vue-next/src/composables/useBLinkHelper.ts:82-83
Timestamp: 2025-05-26T17:27:44.251Z
Learning: In Vue 3.3+, toRef() can accept any input type including raw values, existing refs, and getter functions (MaybeRefOrGetter). It will create a ref from the input or use the existing ref if one is passed. Using toRef() for simple property access is more performant than computed() because it avoids the caching overhead that computeds provide.
Learnt from: VividLemon
PR: #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.
⏰ 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 (11)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (11)
116-150: Excellent refactoring: consolidated sort icon renderingThe replacement of inline sort icon logic with a consistent
SortIconcomponent across all sort states (ascending, descending, default) improves maintainability and ensures visual consistency.
188-188: LGTM: Added necessary Vue imports for SortIcon componentThe addition of
defineComponentandhimports is required for the new programmaticSortIconcomponent implementation.
194-194: LGTM: Added type import for initial sort directionThe
BTableInitialSortDirectiontype import is correctly added to support the new initial sort direction functionality.
229-229: LGTM: Added initialSortDirection prop with sensible defaultThe new
initialSortDirectionprop with default value'asc'enables configurable initial sort direction as specified in the PR objectives.
246-246: LGTM: Added sortCompare prop for custom comparison functionsThe new
sortCompareprop allows table-level custom sort comparison functions, complementing field-level comparers as mentioned in the PR objectives.
312-312: LGTM: Simplified sorted event type signatureRemoving the generic parameter from the 'sorted' event simplifies the API and aligns with the sorting refactoring objectives.
579-579: Good documentation of technical debtThe comment helpfully documents known issues with row styling and accessibility attributes, providing context for future improvements.
793-817: Well-implemented initial sort direction logicThe helper functions correctly handle the new initial sort direction feature:
getLastSortDirection()properly extracts the last used sort directiongetInitialSortDirection()implements the correct precedence: field-level > table-level > default- Support for the special
'last'option is correctly implemented
820-828: Improved resolveOrder function with initial direction supportThe updated
resolveOrderfunction now correctly uses the initial sort direction for both new sorts and whenmustSortforces a direction reset.
832-876: Enhanced sort handling with proper order managementThe updated sort handling functions correctly:
- Use initial sort direction for new field sorting
- Handle undefined orders by removing sort entries from the array
- Maintain proper multi-sort and single-sort behavior
- Return appropriate values for event emission
692-694: Correctly implemented comparer function refactoringThe multi-sort logic now properly uses field-level
sortComparefunctions with fallback to table-levelsortCompareprop, implementing the refactoring objective of moving compare functions from sort-by to fields.
@VividLemon @xvaara - Random implementation question here - I originally extracted this logic out as a template vueuse's So my options as I see them are:
Is there cleaner option that I'm missing? Any strong preference on which way to go? |
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (9)
packages/bootstrap-vue-next/CHANGELOG.md (9)
160-161: Fix typo: corrent → correct (twice)- - **BTabs:** corrent classes on ssr ([2b37c18](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2b37c18dbe64ac430943361e0a67d86206a7e21e)) - - **BTabs:** corrent classes on ssr ([#2664](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2664)) ([2b37c18](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2b37c18dbe64ac430943361e0a67d86206a7e21e)) + - **BTabs:** correct classes on ssr ([2b37c18](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2b37c18dbe64ac430943361e0a67d86206a7e21e)) + - **BTabs:** correct classes on ssr ([#2664](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2664)) ([2b37c18](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2b37c18dbe64ac430943361e0a67d86206a7e21e))
239-251: Fix typos: BDdropdown → BDropdown, propogation → propagation- - **BDdropdown:** Replace click event on split button with split-click ([a9b59c8](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/a9b59c8ee53b7553b9644a4dff90ea4728b77521)) + - **BDropdown:** Replace click event on split button with split-click ([a9b59c8](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/a9b59c8ee53b7553b9644a4dff90ea4728b77521)) - - **BDropdown:** prevent propogation of click events ([3800847](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/380084711e8052e59d83d1ec4ecb85d63e6642c8)) + - **BDropdown:** prevent propagation of click events ([3800847](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/380084711e8052e59d83d1ec4ecb85d63e6642c8))
609-611: Fix typo: failback → fallback- - **BAvatar:** failback on image load error ([#2281](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2281)) ([a51327f](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/a51327f2721aa373329258ff8e551f57c123b394)) + - **BAvatar:** fallback on image load error ([#2281](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2281)) ([a51327f](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/a51327f2721aa373329258ff8e551f57c123b394))
804-804: Fix typo: Fixe → Fix- - **BLink:** Fixe stretch/stretched property issue ([fed1e13](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/fed1e13b40d7b8289883061cbffbce12c4318d87)) + - **BLink:** Fix stretch/stretched property issue ([fed1e13](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/fed1e13b40d7b8289883061cbffbce12c4318d87))
837-837: Component name casing: BFormTextArea → BFormTextareaUse the canonical component name.
- - **BFormInput,BFormTextArea:** Deprecate trim, lazy, number props in favor of modifiers ([#2066](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2066)) ([f5be607](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/f5be6074782b31c3db00b9c46fce0424dd021164)) + - **BFormInput,BFormTextarea:** Deprecate trim, lazy, number props in favor of modifiers ([#2066](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2066)) ([f5be607](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/f5be6074782b31c3db00b9c46fce0424dd021164))
1032-1032: Fix typo: BMOdal → BModal- - **BMOdal:** append transition behavior fixes [#1943](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1943) ([#1947](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1947)) ([e10f742](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/e10f7429fee2c59824933469efe3c66747df3be0)) + - **BModal:** append transition behavior fixes [#1943](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1943) ([#1947](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1947)) ([e10f742](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/e10f7429fee2c59824933469efe3c66747df3be0))
1104-1104: Fix typo: BTableSImple → BTableSimple- - **BTableSImple:** add prop tableAttrs & tableClasses -- previously could not modify table attributes when isResponsive === true ([cf9b942](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/cf9b94214d704e7462ed8ebadf3891e495fbf1f7)) + - **BTableSimple:** add prop tableAttrs & tableClasses -- previously could not modify table attributes when isResponsive === true ([cf9b942](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/cf9b94214d704e7462ed8ebadf3891e495fbf1f7))
1459-1459: Fix typo: pointwer → pointer- - **BPlaceholderButton:** pointwer: wait fixes [#1602](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1602) ([9724aac](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/9724aac227dbd60f7a692e42ff1c6ffdbd9e5153)) + - **BPlaceholderButton:** pointer: wait fixes [#1602](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/1602) ([9724aac](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/9724aac227dbd60f7a692e42ff1c6ffdbd9e5153))
215-215: Fix component name: BFormCheckbboxGroup → BFormCheckboxGroup- - **BFormCheckbboxGroup:** spread the input object rather than el.props fixes [#2590](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2590) ([2c3970c](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2c3970ca3ed0c3d312600877c8ec0043a6834144)) + - **BFormCheckboxGroup:** spread the input object rather than el.props fixes [#2590](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2590) ([2c3970c](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/2c3970ca3ed0c3d312600877c8ec0043a6834144))
🧹 Nitpick comments (10)
packages/nuxt/CHANGELOG.md (1)
1-2: Silence markdownlint MD024 for repeated section headings in changelogsRepeated headings like “Miscellaneous Chores” and “Dependencies” are expected across versions and can trip MD024. Either disable MD024 for this file or set the rule to siblings_only in your markdownlint config.
Apply this minimal, file-local suppression at the top:
+# markdownlint-disable MD024 # ChangelogAlternatively, configure markdownlint (preferred if you have multiple CHANGELOGs):
- In .markdownlint.json:
{ "MD024": { "siblings_only": true } }packages/bootstrap-vue-next/CHANGELOG.md (4)
1-1: Silence markdownlint MD024 for repeated headingsChangelogs commonly repeat headings such as “Features”, “Bug Fixes”, etc. Consider disabling MD024 at the top to avoid noisy CI failures.
+<!-- markdownlint-disable MD024 --> # Changelog
30-31: Deduplicate identical bulletThese two lines report the same change. Keep the richer one (with issue link) and remove the duplicate.
- - **BDropdown:** allow setting icon prop on nested BButton ([018daa5](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/018daa52bb85373f48092ed8a4a46901692645ff)) - **BDropdown:** allow setting icon prop on nested BButton ([#2746](https://github.com/bootstrap-vue-next/bootstrap-vue-next/issues/2746)) ([018daa5](https://github.com/bootstrap-vue-next/bootstrap-vue-next/commit/018daa52bb85373f48092ed8a4a46901692645ff))
91-92: Potential duplicate bulletsThese two bullets appear to describe the same fix, with one linking to issues. Consider collapsing into one for brevity.
1096-1096: Component name casing: Bimg → BImgFor consistency with component naming.
- - **Bimg:** converge props start, end, & center into single "placement" prop + - **BImg:** converge props start, end, & center into single "placement" propapps/docs/src/data/components/table.data.ts (2)
641-646: Clarify 'last' semantics and tighten wording for initialSortDirectionMinor wording improvement to reduce ambiguity for users.
- description: - "Default initial sort direction for all sortable columns that don't specify their own initialSortDirection. Can be 'asc' for ascending, 'desc' for descending, or 'last' to maintain the direction of the previously sorted column.", + description: + "Default initial sort direction for sortable columns that do not specify field.initialSortDirection. Allowed values: 'asc' (ascending), 'desc' (descending), or 'last' (use the previous column’s sort direction when a different column is first sorted).",
729-734: Document comparator contract (negative/zero/positive) and precedenceMake the comparator’s expected return values explicit and reiterate field-level precedence.
- description: - 'A global comparison function for sorting table fields. Field-level sortCompare takes precedence over this table-level setting. The function signature is (a: T, b: T, key: string) => number.', + description: + "A table-level comparison function used when a field does not provide its own 'sortCompare'. Field-level 'sortCompare' takes precedence. Signature: (a: T, b: T, key: string) => number, returning a negative value if a < b, 0 if equal, and a positive value if a > b.",apps/docs/src/docs/components/demo/TableComplete.vue (2)
116-139: Avoid placing interactive content inside BInputGroupTextBInputGroupText is for static text. Place the Clear button directly as a child of BInputGroup to follow Bootstrap 5 patterns.
- <BInputGroupText> - <BButton - :disabled="!filter" - @click="filter = ''" - >Clear</BButton - > - </BInputGroupText> + <BButton + size="sm" + :disabled="!filter" + @click="filter = ''" + > + Clear + </BButton>Optional: add an aria-label to the button if you switch to an icon-only button later.
354-354: Minor typing improvement for filterOn (optional)For stricter TS alignment with
:filterable="filterOn"(expectsstring[]), consider typingfilterOnasref<string[]>([]).Example change in the script (outside the changed lines):
const filterOn = ref<string[]>([])packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
880-948: SortIcon: fix “last” resolution and minor UI polish
- Uses first instead of last when resolving “last” initial direction; mirror the fix applied in handleFieldSorting.
- Using fixed 24px icon size can look off in various contexts; prefer 1em to scale with text.
Apply these diffs:
- if (fieldInitialDirection === 'last') { - const lastSorted = props.sortBy?.find((sort) => sort.order !== undefined) - return lastSorted?.order || 'asc' - } + if (fieldInitialDirection === 'last') { + const lastSorted = [...(props.sortBy ?? [])] + .reverse() + .find((sort) => sort.order !== undefined) + return lastSorted?.order ?? 'asc' + }- 'width': 24, - 'height': 24, + 'width': '1em', + 'height': '1em',Optional nits:
- Consider prefixing a stable class (e.g.,
b-table-sort-icon) in iconClass for easier theming.- Prop name could be
field(instead offieldInfo) to align with the rest of the component API.If you’d like, I can extract SortIcon to BSortIcon.vue with the same API—this improves reuse and makes it testable in isolation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
apps/docs/src/data/components/table.data.ts(2 hunks)apps/docs/src/docs/components/demo/TableComplete.vue(7 hunks)apps/docs/src/docs/components/demo/TableProvider.vue(1 hunks)apps/docs/src/docs/components/demo/TableSortBy.vue(1 hunks)apps/docs/src/docs/components/demo/TableSortByCustom.vue(2 hunks)apps/docs/src/docs/components/table.md(4 hunks)apps/docs/src/docs/migration-guide.md(1 hunks)apps/docs/src/docs/types.md(1 hunks)packages/bootstrap-vue-next/CHANGELOG.md(1 hunks)packages/bootstrap-vue-next/src/components/BModal/BModal.vue(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(11 hunks)packages/bootstrap-vue-next/src/types/ComponentEmits.ts(1 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(2 hunks)packages/bootstrap-vue-next/src/types/index.ts(1 hunks)packages/nuxt/CHANGELOG.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/docs/src/docs/components/demo/TableProvider.vue
- packages/bootstrap-vue-next/src/types/index.ts
- apps/docs/src/docs/components/demo/TableSortBy.vue
- apps/docs/src/docs/components/demo/TableSortByCustom.vue
- packages/bootstrap-vue-next/src/types/ComponentProps.ts
- apps/docs/src/docs/components/table.md
🧰 Additional context used
🧠 Learnings (2)
📚 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:
packages/bootstrap-vue-next/src/types/ComponentEmits.tsapps/docs/src/docs/migration-guide.mdpackages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/CHANGELOG.md
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/types/ComponentEmits.ts (1)
packages/bootstrap-vue-next/src/types/index.ts (1)
BTableSortBy(21-21)
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[grammar] ~762-~762: There might be a mistake here.
Context: ...ed initial-sort-direction for clarity. - The sort icons have been changed - The i...
(QB_NEW_EN)
[grammar] ~764-~764: Ensure spelling is correct
Context: ...d, if you need to customize sorting for localizatio, the documentation on [custom sort comp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
packages/bootstrap-vue-next/CHANGELOG.md
16-16: Multiple headings with the same content
(MD024, no-duplicate-heading)
22-22: Multiple headings with the same content
(MD024, no-duplicate-heading)
28-28: Multiple headings with the same content
(MD024, no-duplicate-heading)
33-33: Multiple headings with the same content
(MD024, no-duplicate-heading)
45-45: Multiple headings with the same content
(MD024, no-duplicate-heading)
60-60: Multiple headings with the same content
(MD024, no-duplicate-heading)
67-67: Multiple headings with the same content
(MD024, no-duplicate-heading)
80-80: Multiple headings with the same content
(MD024, no-duplicate-heading)
89-89: Multiple headings with the same content
(MD024, no-duplicate-heading)
packages/nuxt/CHANGELOG.md
19-19: Multiple headings with the same content
(MD024, no-duplicate-heading)
23-23: Multiple headings with the same content
(MD024, no-duplicate-heading)
33-33: Multiple headings with the same content
(MD024, no-duplicate-heading)
37-37: Multiple headings with the same content
(MD024, no-duplicate-heading)
47-47: Multiple headings with the same content
(MD024, no-duplicate-heading)
51-51: Multiple headings with the same content
(MD024, no-duplicate-heading)
61-61: Multiple headings with the same content
(MD024, no-duplicate-heading)
65-65: Multiple headings with the same content
(MD024, no-duplicate-heading)
75-75: Multiple headings with the same content
(MD024, no-duplicate-heading)
79-79: Multiple headings with the same content
(MD024, no-duplicate-heading)
93-93: Multiple headings with the same content
(MD024, no-duplicate-heading)
🪛 GitHub Check: test-lint
packages/bootstrap-vue-next/src/components/BModal/BModal.vue
[warning] 387-387:
Delete ⏎···
⏰ 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 (11)
packages/nuxt/CHANGELOG.md (1)
7-15: Bullet marker switch and indentation normalization look goodHyphen markers and consistent nesting are valid Markdown and improve visual consistency. No content changes detected.
packages/bootstrap-vue-next/CHANGELOG.md (1)
7-7: Content sanity checkThese entries read clean and consistent for a changelog. No functional impact. LGTM.
Also applies to: 18-18, 24-24, 35-41, 47-52, 58-58, 62-66, 69-74, 82-88, 91-95, 98-98, 104-104, 110-116, 118-119, 121-139
packages/bootstrap-vue-next/src/types/ComponentEmits.ts (1)
51-51: Switch to non-generic BTableSortBy for 'sorted' event payload looks goodThis aligns with the simplification of sort types and matches how the event is described in docs.
If the canonical event name is
update:sort-by(kebab), ensure the migration guide and docs consistently useupdate:sort-byvsupdate:sortBy. Do you want me to sweep the docs and open a follow-up PR to normalize this?apps/docs/src/docs/components/demo/TableComplete.vue (3)
68-74: Nice addition: demoing the new initialSortDirection propClear UX to explore 'asc'/'desc'/'last'. Good showcase.
192-193: Prop wiring LGTMBinding the demo’s
initialSortDirectioninto<BTable :initial-sort-direction="…">correctly exercises the new API.
344-349: Good options set for initial directionThe labels read well and map to the allowed values.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (5)
87-91: SortIcon slot integration looks solidNice encapsulation of the icon logic behind slots while preserving the slot API surface. The precedence of custom slots (
sortAsc/Desc/Default) is maintained and the default fallback works as expected.Also applies to: 102-106, 117-121
200-200: Props additions are appropriate (initialSortDirection, sortCompare)Defaults and typing align with the new behavior. Good job threading both into the sorting flow.
Also applies to: 217-217
547-549: Field-level comparer precedence over table-level is correctResolving comparer as
field?.sortCompare || props.sortComparebefore falling back to localeCompare matches the intended API and prevents sort-by-scoped comparer coupling.
712-718: Single-sort cycle logic LGTMNew sort takes initial direction; subsequent clicks toggle and clear per mustSort rules. The v-model is kept in sync.
159-159: Imports align with new functionalitydefineComponent/h and BTableInitialSortDirection are correctly introduced. No issues.
Also applies to: 165-165
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
699-706: Consistent event payload on multi-sort removalClearing val.order before splicing ensures consumers see a cleared state instead of a stale order. Good catch.
apps/docs/src/docs/migration-guide.md (1)
759-766: Fix spelling and punctuation in BTable sorting bullets
- Correct “localizatio” → “localization”.
- Add periods to bullets for consistency.
- Split the run-on sentence.
Apply this diff:
-Sorting has been significantly reworked. Read the [sorting section](/docs/components/table#sorting) of our documentation. Some specific changes include the following: -- `sort-changed` event is replaced by the `update:sort-by` event -- `sort-direction` has been renamed `initial-sort-direction` for clarity. -- The sort icons have been changed -- The internal `sort-compare` routine has been simplified, if you need to customize sorting for localizatio, the documentation on [custom sort comparers](/docs/components/table#custom-sort-comparers) for details. -- `multi-sort` functionality has been implemented +Sorting has been significantly reworked. Read the [sorting section](/docs/components/table#sorting) of our documentation. Some specific changes include the following: + - `sort-changed` event is replaced by the `update:sort-by` event. + - `sort-direction` has been renamed `initial-sort-direction` for clarity. + - The sort icons have been changed. + - The internal `sort-compare` routine has been simplified. If you need to customize sorting for localization, see the documentation on [custom sort comparers](/docs/components/table#custom-sort-comparers) for details. + - `multi-sort` functionality has been implemented.
🧹 Nitpick comments (2)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (2)
547-550: Comparer precedence and fallback look right; optional micro-optimizations
- Selecting field?.sortCompare over props.sortCompare matches the new API.
- Minor perf nit (optional): inside the sort loop, computedFields.value.find runs per iteration. If desired, cache a Map<key, field> once to reduce lookups to O(1) when sorting large datasets.
Example refactor (optional):
+const fieldByKey = computed(() => { + const map = new Map<string, TableField<Items>>() + for (const f of computedFields.value) if (isTableField<Items>(f)) map.set(f.key, f) + return map +}) ... -const field = computedFields.value.find((f) => f.key === key) +const field = fieldByKey.value.get(key)
883-951: Optional: extract SortIcon to its own component and size with ems
- Extracting to packages/bootstrap-vue-next/src/components/BTable/BSortIcon.vue avoids per-instance definition and encourages reuse across BTable/BTableLite.
- Consider using width/height of 1em so icons scale with font-size, matching typical icon sizing.
Apply this diff to BTable.vue to import and use the extracted component:
-// Simple component for sort icons -const SortIcon = defineComponent({ - props: { /* ...existing props... */ }, - setup(props) { /* ...existing setup... */ } -}) +import BSortIcon from './BSortIcon.vue'And in the template, rename SortIcon tags to BSortIcon accordingly.
New component (BSortIcon.vue):
<script setup lang="ts" generic="Items"> import {computed, h, type PropType} from 'vue' import {type BTableSortBy, type BTableInitialSortDirection, type TableField} from '../../types/TableTypes' const props = defineProps({ fieldInfo: {type: Object as PropType<TableField<Items>>, required: true}, sortBy: {type: Array as PropType<BTableSortBy[] | undefined>, default: undefined}, initialSortDirection: {type: String as PropType<BTableInitialSortDirection>, default: 'asc'}, }) const currentSort = computed(() => props.sortBy?.find((el) => el.key === props.fieldInfo.key)) const ascPath = 'M8 12a.5.5 0 0 0 .5-.5V5.707l2.146 2.147a.5.5 0 0 0 .708-.708l-3-3a.5.5 0 0 0-.708 0l-3 3a.5.5 0 1 0 .708.708L7.5 5.707V11.5a.5.5 0 0 0 .5.5z' const descPath = 'M8 4a.5.5 0 0 1 .5.5v5.793l2.146-2.147a.5.5 0 0 1 .708.708l-3 3a.5.5 0 0 1-.708 0l-3-3a.5.5 0 1 1 .708-.708L7.5 10.293V4.5A.5.5 0 0 1 8 4z' const resolvedDirection = computed(() => { if (currentSort.value?.order) return currentSort.value.order const fieldInitialDirection = props.fieldInfo.initialSortDirection || props.initialSortDirection if (fieldInitialDirection === 'last') { const lastSorted = [...(props.sortBy ?? [])].reverse().find((s) => s.order !== undefined) return lastSorted?.order ?? 'asc' } return fieldInitialDirection === 'desc' ? 'desc' : 'asc' }) const iconPath = computed(() => (resolvedDirection.value === 'desc' ? descPath : ascPath)) const iconClass = computed(() => (resolvedDirection.value === 'desc' ? 'bi bi-arrow-down-short' : 'bi bi-arrow-up-short')) const iconOpacity = computed(() => (currentSort.value?.order ? 1 : 0.4)) </script> <template> <svg xmlns="http://www.w3.org/2000/svg" :width="'1em'" :height="'1em'" fill="currentColor" :class="iconClass" :style="{opacity: iconOpacity}" viewBox="0 0 16 16" aria-hidden="true" > <path fill-rule="evenodd" :d="iconPath" /> </svg> </template>If you prefer to keep it inline for now, at least consider the 1em sizing tweak:
- 'width': 24, - 'height': 24, + 'width': '1em', + 'height': '1em',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/docs/src/docs/migration-guide.md(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(11 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vueapps/docs/src/docs/migration-guide.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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[grammar] ~762-~762: There might be a mistake here.
Context: ...ed initial-sort-direction for clarity. - The sort icons have been changed - The i...
(QB_NEW_EN)
[grammar] ~764-~764: Ensure spelling is correct
Context: ...d, if you need to customize sorting for localizatio, the documentation on [custom sort comp...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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 (10)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (10)
87-91: LGTM: SortIcon integration across header slots is consistent and clearUsing the same SortIcon in sortAsc/sortDesc/sortDefault keeps slot-override behavior intact while unifying visuals. No issues spotted here.
Also applies to: 102-106, 117-121
159-159: LGTM: Import additions are appropriateThe added imports (computed, defineComponent, h, PropType) are all used below. No dead imports detected here.
165-165: Type import aligns with new APIBTableInitialSortDirection import is correct and used where expected.
200-200: Prop default: initialSortDirectionDefaulting to 'asc' is sensible and backwards-compatible.
217-217: Prop default: sortCompareExposing a table-level comparer fallback via prop: good. Field-level override precedence is enforced later.
434-434: Non-functional TODO comment — no action requiredNoted. Once tbodyTrAttrs support lands, wiring aria-selected there would be the right direction.
641-647: Thank you for fixing “last” resolution to actually use the last entryReversing the array before find addresses the earlier “first match” bug and makes the behavior correct for 'last'.
649-667: Initial sort direction resolver is solid
- Field-level initialSortDirection takes precedence over table-level: good.
- 'last' delegates to getLastSortDirection(): consistent with the model behavior.
682-688: LGTM: New-sort path uses initialSortDirectionCreating the initial entry with getInitialSortDirection() aligns with the new configuration.
715-722: LGTM: Single-sort clears model when order is undefinedSetting sortByModel to [] when order resolves to undefined matches expected “click 3rd time to clear” behavior (unless mustSort is active).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (3)
647-653: Fix: “last” initial direction now inspects the last entry (not the first)Using reverse().find(...) correctly retrieves the most recent defined order. This addresses the earlier “first vs last” bug.
674-687: mustSort toggle behavior correctedNow getInitialSortDirection() is only used for new sorts (val === undefined). Existing sorts flip asc<->desc, and when mustSort is disabled, 'desc' can clear to undefined. This resolves the prior toggle bug.
707-715: Multi-sort removal branch correctly emits cleared stateBy setting val.order = undefined before splicing, the emitted payload reflects the cleared state. This fixes the previous stale order emission.
🧹 Nitpick comments (3)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (3)
434-435: A11y: rows should expose aria-selected when selectable rows are usedThe TODO is valid. Without tbodyTrAttrs support, we can’t set ARIA at the row level here. Consider adding tbodyTrAttrs (function or object) to BTableLite and wiring it through so BTable can emit aria-selected on selected rows. Defer if out of scope for this PR.
If you want, I can sketch the minimal changes needed in BTableLite/BTable to support tbodyTrAttrs without breaking existing APIs.
470-475: Nit: tighten Map key typing for fieldByKeyIf TableField.key is typed as string in TableTypes (as it appears elsewhere in this file), prefer Map<string, TableField> to avoid widening to number|symbol unnecessarily.
Apply this small type tightening:
-const fieldByKey = computed(() => { - const map = new Map<string | number | symbol, TableField<Items>>() - for (const f of computedFields.value) if (isTableField<Items>(f)) map.set(f.key, f) - return map -}) +const fieldByKey = computed(() => { + const map = new Map<string, TableField<Items>>() + for (const f of computedFields.value) if (isTableField<Items>(f)) map.set(f.key, f) + return map +})
892-962: Inline SortIcon component is correct; consider extraction for reuse (optional)The component handles:
- Current vs initial direction resolution (including 'last' via reverse().find)
- Visual state via Bootstrap Icons classes and opacity for unsorted vs sorted
It’s accurate and performs well, as it’s instantiated once per BTable instance (not per render). If maintainers prefer consistency and reuse, consider extracting to a dedicated file (e.g., BTableSortIcon.vue) and importing it here. This would also ease unit testing of icon logic.
I can provide a ready-to-commit BTableSortIcon.vue and update the imports/usages here if you want to go that route.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/docs/src/docs/migration-guide.md(1 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.190Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
📚 Learning: 2025-08-18T18:20:08.190Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.190Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vueapps/docs/src/docs/migration-guide.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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vueapps/docs/src/docs/migration-guide.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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
🪛 LanguageTool
apps/docs/src/docs/migration-guide.md
[grammar] ~762-~762: There might be a mistake here.
Context: ...ed initial-sort-direction for clarity. - The sort icons have been changed. - The ...
(QB_NEW_EN)
⏰ 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 (8)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (8)
87-91: SortIcon integration maintains slot compatibility and correctly reflects initial/default statesPassing fieldInfo, sortBy, and table-level initialSortDirection to SortIcon across asc/desc/default keeps backward compatibility with sortAsc/sortDesc/sortDefault slots while unifying visuals. This aligns with the PR goals and looks good.
Also applies to: 102-106, 117-121
159-159: Imports look correct and necessaryAdding defineComponent and h is required for the new SortIcon subcomponent. No issues.
165-165: Type import for initial sort direction is appropriateUsing BTableInitialSortDirection helps keep SortIcon prop typing tight. Good addition.
200-200: New prop default for initialSortDirection ('asc')Defaulting at the table level is sensible and matches the documented behavior. Ensure docs/examples reflect that fields can override this and that 'last' is supported.
217-217: Table-level sortCompare hook addedGood call providing a table-level fallback comparer with field-level override via field.sortCompare. See note below on handling undefined returns from custom comparers.
655-673: Initial direction resolution is correct and prioritized (field > table), including 'last'Clear, predictable behavior. This aligns with migration expectations from bootstrap-vue.
691-697: New-value bootstrap uses initial directionSeeding new entries with getInitialSortDirection() is consistent with the configured behavior. Looks good.
724-731: Single-sort correctly flips/clears and updates the model atomicallyorder resolution and model replacement logic are sound. No issues.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
553-559: Critical: custom comparer result is not validated; fallback to default compare when undefined/NaNIf a custom comparer returns undefined (allowed by typical comparer signatures) or NaN, the comparator can return undefined/NaN to Array.sort, leading to unstable or engine-specific behavior.
Apply this diff to ensure a safe fallback:
- const comparer = field?.sortCompare || props.sortCompare - const comparison = comparer - ? comparer(a, b, key) - : getStringValue(a, key).localeCompare(getStringValue(b, key), undefined, {numeric: true}) + const comparer = field?.sortCompare || props.sortCompare + let comparison: number + if (comparer) { + const c = comparer(a as Items, b as Items, key) + const hasValidNumber = typeof c === 'number' && Number.isFinite(c) + comparison = hasValidNumber + ? c + : getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + } else { + comparison = getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + }Optionally, tighten the BTableSortByComparerFunction type to return number | undefined and document the fallback behavior in the props JSDoc.
♻️ Duplicate comments (1)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
1070-1089: Add explicit assertion for arrow direction (icon), not just opacityThese assertions validate opacity but not arrow orientation. Add checks for the rendered SortIcon class to confirm the direction (bi-arrow-down-short for 'desc', bi-arrow-up-short for 'asc').
You can extend this test or create a small focused one:
it('shows correct sort icon opacity for unsorted columns based on initialSortDirection', () => { @@ - const sortIcons = wrapper.findAll('svg') + const sortIcons = wrapper.findAll('th svg') expect(sortIcons.length).toBeGreaterThan(0) @@ sortIcons.forEach((icon) => { const style = icon.attributes('style') if (style?.includes('opacity')) { expect(style).toContain('opacity: 0.4') } }) }) + + it('shows a descending arrow when initialSortDirection is desc (before click)', () => { + const wrapper = mount(BTable, { + props: {items: simpleItems, fields: simpleFields, initialSortDirection: 'desc' as const}, + }) + const [nameHeader] = wrapper.get('table').findAll('th') + const icon = nameHeader.find('svg') + expect(icon.exists()).toBe(true) + expect(icon.attributes('class')).toContain('bi-arrow-down-short') + })
🧹 Nitpick comments (7)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (3)
838-854: Fallback to default comparer is verified; consider adding “undefined” return fallback testCurrent test ensures default localeCompare path works. To guard against custom comparers returning undefined (or NaN), add a test ensuring fallback to default comparison occurs. This will also protect against regressions once the comparator fallback is implemented in BTable.vue.
Proposed test to append in this suite:
it('falls back to default comparison when no custom comparer provided', () => { @@ expect(text).toStrictEqual(['Alice', 'Bob', 'Charlie', 'David']) }) + + it('falls back to default when custom comparer returns undefined', () => { + const fieldsWithUndefComparer: Exclude<TableField<SimplePerson>, string>[] = [ + { + key: 'first_name', + label: 'First Name', + sortable: true, + // Intentionally return undefined to trigger fallback + sortCompare: () => undefined as unknown as number, + }, + {key: 'age', label: 'Age', sortable: true}, + ] + const wrapper = mount(BTable, { + props: { + items: customSortItems, + fields: fieldsWithUndefComparer, + sortBy: [{order: 'asc', key: 'first_name'}], + }, + }) + const text = wrapper + .get('tbody') + .findAll('tr') + .map((row) => row.find('td').text()) + expect(text).toStrictEqual(['Alice', 'Bob', 'Charlie', 'David']) + })
1000-1019: Fix misleading test comment (“asc” vs “desc”)The inline comment says the last sort direction from 'age' is asc, but the provided sortBy is desc. The expectation also asserts desc order. Update the comment to avoid confusion.
- // Click on name field - should use the last sort direction from 'age' (asc), not itself (desc) + // Click on name field - should use the last sort direction from 'age' (desc), not its own default
1021-1049: Fix misleading test comment (“asc” vs “desc”)Same mismatch as above. sortBy starts with age desc; expectation is descending name order.
- // Click on name field - should use the last sort direction from 'age' (asc), not itself (desc) + // Click on name field - should use the last sort direction from 'age' (desc), not its own defaultpackages/bootstrap-vue-next/src/components/BTable/BTable.vue (4)
87-91: Inline SortIcon usage works; consider extraction to a dedicated componentThe inline SortIcon is functionally correct and keeps templates DRY. However, defining it inline recreates the component on parent re-renders and makes reuse harder.
You can extract SortIcon to src/components/BTable/BSortIcon.vue and import it here. This aligns with composability and reuse. The render and props API can remain identical to minimize churn.
Also applies to: 102-106, 117-121
675-700: Resolve-order logic works but is over-coupled to initial direction; simplify toggle behaviorCurrent toggle uses initial direction to decide asc->desc vs asc->undefined transitions. That passes current tests but makes the behavior harder to reason about and maintain. Initial direction should only apply for new sorts (val === undefined). Thereafter, toggle asc<->desc and clear to undefined only when mustSort does not apply.
Apply this simplification:
- const resolveOrder = (val: BTableSortByOrder | undefined): BTableSortByOrder | undefined => { - // New sort: honor the configured initial direction - if (val === undefined) return getInitialSortDirection() - // Determine initial direction for this field - const initial = getInitialSortDirection() - const must = - props.mustSort === true || - (Array.isArray(props.mustSort) && props.mustSort.includes(fieldKey as string)) - console.log('val', val, 'initial', initial, 'must', must) - if (val === 'asc') { - if (initial === 'desc') { - // If mustSort, cycle asc -> desc, else asc -> undefined - return must ? 'desc' : undefined - } - // If initial is asc, cycle asc -> desc -> undefined (or asc if mustSort) - return 'desc' - } - if (val === 'desc') { - if (initial === 'desc') { - return 'asc' - } - // If mustSort, cycle desc -> asc, else desc -> undefined - return must ? 'asc' : undefined - } - return undefined - } + const resolveOrder = (val: BTableSortByOrder | undefined): BTableSortByOrder | undefined => { + // New sort: honor the configured initial direction + if (val === undefined) return getInitialSortDirection() + const must = + props.mustSort === true || + (Array.isArray(props.mustSort) && props.mustSort.includes(fieldKey as string)) + // Toggle behavior + if (val === 'asc') return 'desc' + if (val === 'desc') return must ? 'asc' : undefined + return undefined + }This keeps the “new sort” behavior intact and yields predictable toggling thereafter.
905-975: SortIcon logic is correct and consistent with “last” semantics
- Uses reversed scan for 'last'.
- Applies field-level initialSortDirection override.
- Reflects current sort with opacity and class.
Consider deduplicating 'last' resolution logic with handleFieldSorting to avoid divergence in future maintenance (e.g., small util).
You could extract:
// outside components const resolveInitialDirectionForField = ( field: Pick<TableField, 'initialSortDirection' | 'key'>, tableInitial: BTableInitialSortDirection | undefined, sortBy: BTableSortBy[], excludeKey?: string | number | symbol ): BTableSortByOrder => { const getLast = (): BTableSortByOrder => [...(sortBy ?? [])].reverse().find((s) => s.order !== undefined && s.key !== excludeKey)?.order ?? 'asc' const init = field.initialSortDirection ?? tableInitial ?? 'asc' return init === 'last' ? getLast() : init === 'desc' ? 'desc' : 'asc' }Then reuse it in both SortIcon and handleFieldSorting.
434-435: Note on accessibility: aria-selected on selected rowsThe TODO is valid. When you get to implementing tbodyTrAttrs support, ensure selected rows expose aria-selected="true" for a11y parity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)packages/bootstrap-vue-next/src/components/BTable/table.spec.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
📚 Learning: 2025-08-18T18:20:08.240Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/table.spec.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/table.spec.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 Learning: 2025-08-20T14:05:32.372Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2732
File: packages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue:28-28
Timestamp: 2025-08-20T14:05:32.372Z
Learning: Vue 3 supports TypeScript annotations in template binding expressions when using `<script setup lang="ts">` or `<script lang="ts">`. Template ref callbacks like `:ref="(ref: ComponentPublicInstance) => ..."` are valid TypeScript syntax in Vue templates when TypeScript is enabled in the script block.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
🧬 Code Graph Analysis (1)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
packages/bootstrap-vue-next/src/types/TableTypes.ts (2)
TableItem(9-13)TableField(56-82)
🪛 GitHub Check: test-lint
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
[warning] 683-683:
Unexpected console statement
⏰ 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 (13)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (6)
755-794: Solid coverage for table-level comparer overrideThe test clearly demonstrates table-level sortCompare reversing the default order. Assertions match expected DOM order.
796-837: Correct precedence of field-level comparer over table-levelGood job asserting that the field-level comparer wins. The test verifies both key-specific logic and fallback.
856-891: Good combined scenario test (custom comparer + initial direction)This validates that initial sort direction and comparer interplay works as expected on first click. Nice.
893-935: Multi-sort with mixed comparers is well coveredVerifies stable grouping and reversed intra-group sort correctly. Looks good.
1129-1171: mustSort=true cycling test is precise and correctValidates desc -> asc -> desc cycling and aria-sort accuracy. Good coverage.
1172-1219: mustSort unset cycling test is correctVerifies desc -> asc -> cleared model. Good assertions including aria-sort neutral state and original order.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (7)
200-200: Default initialSortDirection prop is a sensible choiceSetting 'asc' as the default keeps behavior backward-compatible for most consumers while enabling field-level overrides.
217-217: sortCompare prop declared; ensure comparator fallback is implementedDeclaring sortCompare at the table level is consistent with the new API. See comparator fallback note below to handle undefined/NaN returns from custom comparers.
470-475: Efficient field lookup via MapfieldByKey avoids repeated scans during sort. Good for performance.
647-655: Correct “last” direction resolution (excludes current column and inspects last entry)This addresses the earlier “first vs last” find issue and excludes the current field when computing “last”, which is correct in multi-sort scenarios.
656-674: Initial sort direction resolution is well-structuredField-level overrides table-level; “last” uses getLastSortDirection; fallback to 'asc'. Looks good.
705-745: Sorted event payload consistency is handled correctlyMulti-sort branch clears val.order before removal; single-sort branch clears the model when order resolves to undefined. Matches prior feedback. LGTM.
891-903: ariaBusy passthrough is fine; ensure it renders aria-busy on tableThe test targets ariabusy; double-check BTableLite maps ariaBusy to aria-busy. If it doesn’t, we should align attribute casing in BTableLite.
Would you like me to scan BTableLite.vue for the final table attribute name?
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
553-559: Critical: custom comparer returning undefined/NaN should fall back to default compareCurrent code passes through undefined (or NaN when negated), causing invalid comparator outputs and unstable sorts when a user-defined comparer chooses to “defer” to default comparison.
Apply this safe fallback:
- const comparer = field?.sortCompare || props.sortCompare - const comparison = comparer - ? comparer(a, b, key) - : getStringValue(a, key).localeCompare(getStringValue(b, key), undefined, {numeric: true}) + const comparer = field?.sortCompare || props.sortCompare + let comparison: number + if (comparer) { + const c = comparer(a, b, key) + comparison = + typeof c === 'number' && Number.isFinite(c) + ? c + : getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + } else { + comparison = getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + }
🧹 Nitpick comments (4)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (4)
434-435: A11y: expose aria-selected on rows when selectedYou’ve noted this TODO; agree it’s worth addressing. Even a minimal boolean attr on the TR (or via tbodyTrAttrs when available) will help assistive tech.
If you want, I can draft a minimal tbodyTrAttrs hook (plus tests) behind a non-breaking prop to carry aria-selected until tbodyTrAttrs is fully supported.
470-475: Map fields by key for O(1) lookup — goodUsing a Map to resolve field-level sortCompare is the right call. Minor nit: BTableSortBy.key is typed as string, so narrowing the Map key type to string reduces cognitive overhead.
Apply this diff if you want to narrow types:
-const fieldByKey = computed(() => { - const map = new Map<string | number | symbol, TableField<Items>>() +const fieldByKey = computed(() => { + const map = new Map<string, TableField<Items>>() for (const f of computedFields.value) if (isTableField<Items>(f)) map.set(f.key, f) return map })
647-674: 'last' direction logic is correct and excludes the current columnReverse-searching the model and skipping the current field fixes the “first vs. last” bug and produces the expected semantics for 'last'. Nicely done.
If your TS target supports it, Array.prototype.findLast would remove the need to allocate and reverse:
const lastSorted = sortByModel.value?.findLast((s) => s.order !== undefined && s.key !== fieldKey)
904-974: SortIcon: solid logic; consider width/height and potential extraction
- Logic: resolvedDirection handles 'last' correctly and mirrors table-level behavior. Good.
- Sizing: hardcoding width/height 24 can break alignment with Bootstrap icons. Prefer 1em to match font size and surrounding layout.
- Extraction: optional, but moving SortIcon to its own .vue (e.g., BSortIcon.vue) avoids recreating the component options on parent renders and improves reusability across table variants.
Adjust sizing to be font-relative:
- 'width': 24, - 'height': 24, + 'width': '1em', + 'height': '1em',If maintainers are OK with a small pattern addition, extract SortIcon into packages/bootstrap-vue-next/src/components/BTable/BSortIcon.vue and import it here. This keeps BTable.vue slimmer and lets other table-likes reuse the icon without duplicating logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue(12 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
📚 Learning: 2025-08-18T18:20:08.240Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 Learning: 2025-08-20T14:05:32.372Z
Learnt from: xvaara
PR: bootstrap-vue-next/bootstrap-vue-next#2732
File: packages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue:28-28
Timestamp: 2025-08-20T14:05:32.372Z
Learning: Vue 3 supports TypeScript annotations in template binding expressions when using `<script setup lang="ts">` or `<script lang="ts">`. Template ref callbacks like `:ref="(ref: ComponentPublicInstance) => ..."` are valid TypeScript syntax in Vue templates when TypeScript is enabled in the script block.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
⏰ 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 (7)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (7)
87-121: Good move to a dedicated SortIcon; preserves slot API while centralizing logicRendering the same SortIcon across sortAsc/sortDesc/sortDefault slots keeps backward-compat with per-slot overrides and consolidates direction logic into one place. Looks solid.
159-159: Import additions are appropriatedefineComponent/h imports and PropType are needed for SortIcon and TS props typing. All good.
165-165: Type import for initial sort direction aligns with new APIUsing BTableInitialSortDirection keeps types explicit for 'asc' | 'desc' | 'last'. Good.
200-201: New props: initialSortDirection and sortCompare defaults are sensible
- initialSortDirection default 'asc' matches historical expectations.
- sortCompare default undefined properly defers to built-in compare.
Also applies to: 217-217
701-709: New-sort behavior uses initial direction as intendedUsing getInitialSortDirection for first-time sorts (including 'last') aligns with the API and UX goals.
713-731: Multi-sort removal branch emits a cleared payload — goodSetting val.order = undefined before removal keeps the emitted 'sorted' payload consistent with the new model state.
736-744: Single-sort branch correctly clears model when order resolves to undefinedThis keeps both the model and emitted value consistent. Looks good.
|
@VividLemon - I believe this is ready to go as well. But let me know if there is anything you want on this (I know we had a bunch of loops on the name of initia-sort-direction, but whatever the name, I'm convinced that it's functionality that BSV had that we should include in BSVN). Also, any thoughts on this (which was not affected by this PR, but coderabittai brought up):
|
Is the helper component public or private? Exposed to consumers? |
The helper component is private - it's just a way to encapsulate the sort icons/logic so that we don't have duplicate svgs for these in a bunch of places. |
In that case, helper components are included in the package. The way I had been doing them is all public artifacts go into their own special folders, while private utility artifacts are held in the root. So This was done after createReusableTemplate was introduced, but createReusableTemplate may be better off removed in favor of real SFC's |
Thanks. I've refactored into ../components/SortIcon.vue for my private helper component. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
553-559: Critical: custom sortCompare returning undefined/NaN yields invalid comparator results.When sortCompare returns undefined or NaN, Array.sort receives NaN (or undefined), leading to unstable or engine-dependent sorts. Fall back to the default comparer when the custom comparer result isn’t a finite number.
Apply this diff:
- const comparer = field?.sortCompare || props.sortCompare - const comparison = comparer - ? comparer(a, b, key) - : getStringValue(a, key).localeCompare(getStringValue(b, key), undefined, {numeric: true}) + const comparer = field?.sortCompare || props.sortCompare + let comparison: number + if (comparer) { + const c = comparer(a, b, key as string) + comparison = Number.isFinite(c as number) + ? (c as number) + : getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + } else { + comparison = getStringValue(a, key).localeCompare( + getStringValue(b, key), + undefined, + {numeric: true} + ) + }To solidify behavior, add a spec that uses a field-level sortCompare returning undefined and NaN, asserting the fallback ordering is applied.
Run this to check whether such a test already exists:
#!/bin/bash rg -nP -C3 --glob '**/*.{test,spec}.ts*' '(sortCompare).*return(s)?\s+(undefined|NaN)'
🧹 Nitpick comments (4)
packages/bootstrap-vue-next/src/components/SortIcon.vue (2)
4-11: Use scalable, font-relative sizing for the SVG (improves consistency across themes).The icon’s viewBox is 16x16 but width/height are 24px, which can look oversized next to text. Prefer 1em so it inherits the font size and stays aligned with Bootstrap icon sizing.
Apply this diff:
- width="24" - height="24" + width="1em" + height="1em" fill="currentColor" :class="iconClass" :style="{opacity: iconOpacity}" viewBox="0 0 16 16" aria-hidden="true" + focusable="false"
48-50: Add project-scoped classes to avoid implicit dependency on bootstrap-icons CSS.The SVG path is inlined, so the “bi …” classes aren’t required for rendering. Add a stable project prefix to improve theming without forcing bootstrap-icons CSS.
Apply this diff:
-const iconClass = computed(() => - resolvedDirection.value === 'desc' ? 'bi bi-arrow-down-short' : 'bi bi-arrow-up-short' -) +const iconClass = computed(() => + resolvedDirection.value === 'desc' + ? 'b-table-sort-icon is-desc bi bi-arrow-down-short' + : 'b-table-sort-icon is-asc bi bi-arrow-up-short' +)Optionally document b-table-sort-icon, .is-asc, .is-desc in the component’s README or the table docs for users who want to theme the icon.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (2)
703-709: Minor DRY: use resolveOrder(undefined) for new entries.Slightly simplifies updatedValue initialization and guarantees a single source of truth for initial direction.
Apply this diff:
- const updatedValue: BTableSortBy = - // If value is new, we use the field's initialSortDirection or default to ascending - // Otherwise we make a temp copy of the value - index === -1 || !originalValue - ? {key: fieldKey as string, order: getInitialSortDirection()} - : {...originalValue} + const updatedValue: BTableSortBy = + index === -1 || !originalValue + ? {key: fieldKey as string, order: resolveOrder(undefined)} + : {...originalValue}
77-123: Reduce duplicated template for sort icon slots (keep slot API intact).The three slot blocks (sortAsc/sortDesc/sortDefault) contain identical SortIcon markup. You can compute the slot name once and render a single to eliminate duplication without breaking the public slot API.
Example refactor (conceptual):
<!-- inside the same v-if block --> <slot v-bind="scope" :name="(() => { const order = sortByModel?.find((el) => el.key === scope.field.key)?.order return order === 'asc' ? (slots[`sortAsc(${String(scope.field.key)})`] ? `sortAsc(${String(scope.field.key)})` : 'sortAsc()') : order === 'desc' ? (slots[`sortDesc(${String(scope.field.key)})`] ? `sortDesc(${String(scope.field.key)})` : 'sortDesc()') : (slots[`sortDefault(${String(scope.field.key)})`] ? `sortDefault(${String(scope.field.key)})` : 'sortDefault()') })() as 'sortAsc()' | 'sortDesc()' | 'sortDefault()'" > <SortIcon :field-info="scope.field" :sort-by="sortByModel" :initial-sort-direction="props.initialSortDirection" /> </slot>This keeps full backward compatibility while avoiding triple maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue(11 hunks)packages/bootstrap-vue-next/src/components/SortIcon.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/bootstrap-vue-next/src/components/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place all new Vue components under packages/bootstrap-vue-next/src/components/
Files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/SortIcon.vue
packages/bootstrap-vue-next/src/components/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep component-specific styles inside the component .vue files
Files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/SortIcon.vue
🧠 Learnings (3)
📓 Common learnings
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
📚 Learning: 2025-08-18T18:20:08.240Z
Learnt from: dwgray
PR: bootstrap-vue-next/bootstrap-vue-next#2777
File: packages/bootstrap-vue-next/CHANGELOG.md:11-11
Timestamp: 2025-08-18T18:20:08.240Z
Learning: For PR #2777 (BSort updates), keep changes scoped to BTable sorting work; unrelated edits like CHANGELOG typos should be deferred to a separate PR/issue.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
packages/bootstrap-vue-next/src/components/BTable/BTable.vue
⏰ 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 (5)
packages/bootstrap-vue-next/src/components/SortIcon.vue (1)
38-41: LGTM: correct “last” semantics (searching from the end).Using reverse().find(...) ensures “last” reflects the most recently applied sort order. Nice catch and consistent with BTable’s logic.
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (4)
647-674: LGTM: initial “last” direction now inspects the last sorted entry and excludes the current column.getLastSortDirection reverses the model and ignores the active field, which avoids self-referential toggles. getInitialSortDirection correctly resolves field-level first, then table-level, then defaults to 'asc'.
675-699: LGTM: mustSort + toggle logic matches expected cycles.resolveOrder now uses the initial direction only for new sorts and correctly flips asc<->desc for existing sorts, clearing when allowed. This addresses the previously raised stuck-on-initial issue.
164-164: LGTM: SortIcon extracted as a private helper component under src/components.Placement follows repo conventions for private utilities; improves reusability and avoids re-creation on parent re-renders.
200-201: LGTM: new public props defaults.Adding initialSortDirection (default 'asc') and sortCompare (default undefined) is consistent with the feature objectives and types.
Also applies to: 217-218
3 participants
Describe the PR
This fixes two of the outstanding issues in the sorting functionality of BTable
initial-sort-directionon fields or table - Fixes BTable: Not possible to set initial sort direction of fields to descending #2775comparefunction out ofTableSortBytoTableFieldandBTable, rename tosortCompareto bring it back in line with BSV (and since it's now inline with other non-sort-related props Fixes table comparer function sortby #2599Small replication
The full example in the table docs
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)feat(...)fix(...)docs(...)The PR fulfills these requirements:
CHANGELOGis 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
New Features
UI
Demos
Documentation
Tests