refactor(BTable): move code into composables for better sorting #2913
Conversation
feat(BTable): Enhanced BTableEmits and BTableProps to use readonly array types for better immutability
|
|
WalkthroughThis PR updates module resolution by converting absolute import paths to relative paths, standardizes dependency injection with explicit null defaults across composables and components, introduces comprehensive table utility composables ( Changes
Sequence DiagramsequenceDiagram
participant Consumer
participant Table as BTable
participant useTableHelpers
participant useTableMapper
participant useTableSelectedItems
participant Provider
rect rgb(200, 220, 255)
Note over Consumer,Provider: Table Initialization & Data Flow
Consumer->>Table: Mount with props (fields, items, sortBy, etc.)
Table->>useTableHelpers: useTableMapper(...config)
useTableMapper->>useTableMapper: Resolve fields, compute headers
useTableMapper->>useTableMapper: Build formatters per field
useTableMapper->>useTableMapper: Apply sort/filter/pagination
useTableHelpers-->>Table: { items, displayItems, fields }
end
rect rgb(220, 240, 220)
Note over Consumer,Provider: Selection Flow
Consumer->>Table: Click row / Select via API
Table->>useTableSelectedItems: selectRow(item, modifiers)
useTableSelectedItems->>useTableSelectedItems: Update selection state
useTableSelectedItems->>Table: Emit 'row-selected' event
useTableSelectedItems-->>Table: { exposed: { selectRow, clearSelected, isRowSelected, ... } }
Table->>Consumer: Slot binding with selectedItemsUtilities.exposed
end
rect rgb(255, 240, 200)
Note over Consumer,Provider: Provider-backed Data
Table->>useTableHelpers: callItemsProvider() on filter/sort/page change
useTableHelpers->>Provider: Invoke provider({ sortBy, filter, pagination, signal })
Provider-->>useTableHelpers: MaybePromise<readonly Items[] | undefined>
useTableHelpers->>useTableHelpers: Debounce, set busy state, emit 'provider-changed'
useTableHelpers-->>Table: Updated items
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/bootstrap-vue-next/src/composables/useRtl.ts (1)
28-28: Fix the return type to handle null rtlPlugin properly.When
rtlPluginisnull(which is the default injection value), spreading it with{...rtlPlugin}results in an empty object{}. Consumers destructuring{isRtl, locale}receiveundefinedvalues instead of the expected structure. While the current consumer in BFormSpinbutton.vue handles this with optional chaining (?.value), this is fragile and violates the return type contract.Location:
packages/bootstrap-vue-next/src/composables/useRtl.ts, line 28The return statement should provide a default structure when
rtlPluginis null, rather than spreading a null value. Consider returningrtlPlugindirectly (if it's already properly typed) or a merged default object that ensures the expected properties are always present in the correct structure.packages/bootstrap-vue-next/src/composables/useModalManager.ts (1)
54-57: Fix the spread operator to handle null modalManagerPlugin.Spreading
nullthrows a TypeError. The current code at lines 54-57:return { ...modalManagerPlugin, dispose, }will crash when
modalManagerPluginis null (injected with null fallback at line 16). The code uses optional chaining elsewhere (lines 22-23, 41, 49), indicating null is an expected scenario. The return statement should either provide a fallback object or conditionally spread:return { ...(modalManagerPlugin || {}), dispose, }This ensures callers receive the
disposefunction and safe access to modal stack properties.packages/bootstrap-vue-next/src/composables/useShowHide.ts (1)
502-539: Restore consumer precedence when mergingtransitionProps.By keeping
options.transitionPropsinsidetransitionFunctionsand then spreadingtransitionFunctionsafterprops.transitionProps, internal defaults now override any user-supplied transition settings (e.g.css,mode, durations). Previously consumers could override component defaults; this refactor regresses that capability. Please move theoptions.transitionPropsspread out oftransitionFunctionsand apply it before the user spread so external overrides win while the hook arrays continue to fan-in both sources.- const transitionFunctions = { - ...options.transitionProps, + const transitionFunctions = { onBeforeEnter, onEnter, onAfterEnter, onBeforeLeave, onLeave, onAfterLeave, } satisfies TransitionProps return { showRef: readonly(showRef), @@ - transitionProps: { - ...fadeBaseTransitionProps, - ...props.transitionProps, - ...transitionFunctions, - } satisfies TransitionProps, + transitionProps: { + ...fadeBaseTransitionProps, + ...options.transitionProps, + ...props.transitionProps, + ...transitionFunctions, + } satisfies TransitionProps,packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue (1)
110-123: Default cell slot exposesitemsas undefined; should passprops.itemsIn the default
cell()slot, you expose:items="items":<slot ... :item="item" :field="field" :items="items" :toggle-details="() => toggleRowDetails(item)" :details-showing="isTableItem(item) ? (getDetailsValue(item) ?? false) : false" >With
<script setup>you only haveprops(viauseDefaults) and no top-levelitemsbinding, soitemshere will beundefinedat runtime. That breaks the slot contract for consumers expecting the full items array onslotProps.items.You likely want to forward
props.itemsinstead:- :item="item" - :field="field" - :items="items" + :item="item" + :field="field" + :items="props.items"
🧹 Nitpick comments (2)
packages/bootstrap-vue-next/src/utils/tableUtils.ts (1)
64-70: Type alias extraction and helper look good; consider derivingStackedPropsfrom prop typesThis refactor keeps
getDataLabelAttrbehavior intact and makes the signature cleaner by centralizing the stacked-related props inStackedProps.To avoid type drift if the
stackedorlabelStackedprop types evolve, you might consider definingStackedPropsin terms of the actual prop types, e.g.:// Example sketch – adjust to actual prop definitions export type StackedProps = Readonly< Pick<BTableLiteProps<unknown>, 'stacked' | 'labelStacked'> >That way any future changes to those props automatically propagate here, and you also get readonly semantics consistent with prop types.
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue (1)
218-218: Generic parameter naming (Items) vs preferredItemThe script uses
generic="Items"andBTableLiteProps<Items>. Prior learnings for this repo recommend using the singularItemfor the row generic (e.g.,Item/Item[]) to improve readability and consistency across table types. This is stylistic and not blocking, but when you next touch the generics for BTable/BTableLite, it may be worth aligning toItemfor consistency.
Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/docs/src/components/PageContents.vue(1 hunks)apps/docs/src/components/PageContentsItem.vue(1 hunks)apps/docs/src/data/components/accordion.data.ts(1 hunks)apps/docs/src/data/components/app.data.ts(1 hunks)apps/docs/src/data/components/listGroup.data.ts(1 hunks)apps/docs/src/data/components/orchestrator.data.ts(1 hunks)packages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue(1 hunks)packages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vue(1 hunks)packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue(2 hunks)packages/bootstrap-vue-next/src/components/BPopover/BPopover.vue(2 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(8 hunks)packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue(6 hunks)packages/bootstrap-vue-next/src/components/SortIcon.vue(1 hunks)packages/bootstrap-vue-next/src/composables/orchestratorShared.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useBreadcrumb/index.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useModal/index.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useModalManager.ts(1 hunks)packages/bootstrap-vue-next/src/composables/usePopover/index.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useProvideDefaults/index.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useRegistry/index.ts(3 hunks)packages/bootstrap-vue-next/src/composables/useRtl.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useShowHide.ts(6 hunks)packages/bootstrap-vue-next/src/composables/useTableHelpers.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useTableLiteHelpers.ts(1 hunks)packages/bootstrap-vue-next/src/composables/useToast/index.ts(1 hunks)packages/bootstrap-vue-next/src/types/ComponentEmits.ts(2 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(6 hunks)packages/bootstrap-vue-next/src/types/TableTypes.ts(3 hunks)packages/bootstrap-vue-next/src/utils/debounce.ts(1 hunks)packages/bootstrap-vue-next/src/utils/functions.ts(1 hunks)packages/bootstrap-vue-next/src/utils/tableUtils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/bootstrap-vue-next/src/components/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and modify Vue components only under packages/bootstrap-vue-next/src/components/
Files:
packages/bootstrap-vue-next/src/components/BPopover/BPopover.vuepackages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vuepackages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
packages/bootstrap-vue-next/src/components/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep component-specific styles within their respective .vue single-file components
Files:
packages/bootstrap-vue-next/src/components/BPopover/BPopover.vuepackages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vuepackages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
apps/docs/src/data/components/*.data.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
apps/docs/src/data/components/*.data.ts: When adding or modifying component props, events, or slots, ALWAYS update the corresponding .data.ts file in apps/docs/src/data/components/
Documentation format in .data.ts files must match TypeScript interfaces exactly (props, emits, slots definitions)
Files:
apps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/listGroup.data.tsapps/docs/src/data/components/accordion.data.ts
🧠 Learnings (27)
📓 Common learnings
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 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.
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next 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
Repo: bootstrap-vue-next/bootstrap-vue-next 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: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/App.vue : Use packages/bootstrap-vue-next/src/App.vue as a local test area for component changes
📚 Learning: 2025-09-30T23:57:21.526Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2866
File: apps/docs/src/docs/components/demo/ModalModel.vue:11-15
Timestamp: 2025-09-30T23:57:21.526Z
Learning: The bootstrap-vue-next documentation project uses unplugin-vue-components to automatically resolve and import Vue components like BModal, BButton, etc. Explicit imports for these components in script sections are not required.
Applied to files:
packages/bootstrap-vue-next/src/composables/useModalManager.tspackages/bootstrap-vue-next/src/utils/debounce.tspackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.tspackages/bootstrap-vue-next/src/composables/useRegistry/index.tspackages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-04-28T22:48:46.738Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/composables/usePopover/index.tspackages/bootstrap-vue-next/src/components/BPopover/BPopover.vueapps/docs/src/data/components/orchestrator.data.tspackages/bootstrap-vue-next/src/composables/useModal/index.tspackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.tspackages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue
📚 Learning: 2025-06-26T19:46:19.333Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2762
File: apps/docs/src/docs/components/tooltip.md:0-0
Timestamp: 2025-06-26T19:46:19.333Z
Learning: BTooltip is a very thin wrapper around BPopover in bootstrap-vue-next. There is no separate `useTooltipController` composable - the `usePopoverController` composable can be used to programmatically control both popovers and tooltips.
Applied to files:
packages/bootstrap-vue-next/src/composables/usePopover/index.tspackages/bootstrap-vue-next/src/components/BPopover/BPopover.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/components/index.ts : When adding a new component, update packages/bootstrap-vue-next/src/components/index.ts to export it
Applied to files:
packages/bootstrap-vue-next/src/utils/debounce.tspackages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/types/ComponentEmits.tsapps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/utils/functions.tspackages/bootstrap-vue-next/src/composables/useTableLiteHelpers.tspackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.tspackages/bootstrap-vue-next/src/composables/useRegistry/index.tspackages/bootstrap-vue-next/src/composables/useTableHelpers.tspackages/bootstrap-vue-next/src/composables/useShowHide.ts
📚 Learning: 2025-10-21T19:31:54.113Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2882
File: apps/docs/src/docs/components/demo/PlaceholderWidth.vue:1-22
Timestamp: 2025-10-21T19:31:54.113Z
Learning: The bootstrap-vue-next project uses unplugin to automatically import components, so explicit imports in demo/documentation components are not needed.
Applied to files:
packages/bootstrap-vue-next/src/utils/debounce.tspackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/App.vue : Use packages/bootstrap-vue-next/src/App.vue as a local test area for component changes
Applied to files:
packages/bootstrap-vue-next/src/utils/debounce.tspackages/bootstrap-vue-next/src/components/BPopover/BPopover.vuepackages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vueapps/docs/src/components/PageContentsItem.vuepackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.tspackages/bootstrap-vue-next/src/composables/useRegistry/index.tspackages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vuepackages/bootstrap-vue-next/src/composables/useTableHelpers.tspackages/bootstrap-vue-next/src/composables/useShowHide.tspackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-08-19T23:30:07.062Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2762
File: apps/docs/src/docs/components/tooltip.md:90-94
Timestamp: 2025-08-19T23:30:07.062Z
Learning: In bootstrap-vue-next docs, tooltip.data.ts uses popoverSharedProps('popover') to define props, which includes the 'manual' prop through the shared props structure from popover-shared.ts. The manual prop is available in BTooltip documentation through this prop spreading/inheritance mechanism.
Applied to files:
packages/bootstrap-vue-next/src/components/BPopover/BPopover.vueapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.tspackages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vue
📚 Learning: 2025-08-19T23:30:07.062Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2762
File: apps/docs/src/docs/components/tooltip.md:90-94
Timestamp: 2025-08-19T23:30:07.062Z
Learning: In bootstrap-vue-next docs, tooltip.data.ts uses popoverSharedProps('tooltip') to define props, which includes the 'manual' prop through the shared props structure from popover-shared.ts. The manual prop is available in BTooltip documentation through this inheritance mechanism.
Applied to files:
packages/bootstrap-vue-next/src/components/BPopover/BPopover.vueapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-05-28T07:32:45.658Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 Learning: 2025-08-18T18:20:08.240Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 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/SortIcon.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/types/TableTypes.tspackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-04-24T20:35:48.629Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next 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`.
Applied to files:
packages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/types/ComponentEmits.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/composables/useTableLiteHelpers.tspackages/bootstrap-vue-next/src/utils/tableUtils.tsapps/docs/src/components/PageContentsItem.vuepackages/bootstrap-vue-next/src/types/ComponentProps.tspackages/bootstrap-vue-next/src/types/TableTypes.tspackages/bootstrap-vue-next/src/composables/useTableHelpers.tspackages/bootstrap-vue-next/src/components/BTable/BTableLite.vueapps/docs/src/components/PageContents.vue
📚 Learning: 2025-08-19T14:23:46.775Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2732
File: packages/bootstrap-vue-next/CHANGELOG.md:35-41
Timestamp: 2025-08-19T14:23:46.775Z
Learning: In the bootstrap-vue-next repository, CHANGELOG.md files (e.g., packages/bootstrap-vue-next/CHANGELOG.md) are autogenerated and should be ignored in reviews; do not propose edits for them.
Applied to files:
packages/bootstrap-vue-next/src/components/SortIcon.vuepackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to apps/docs/src/data/components/*.data.ts : Documentation format in .data.ts files must match TypeScript interfaces exactly (props, emits, slots definitions)
Applied to files:
packages/bootstrap-vue-next/src/types/ComponentEmits.tsapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/listGroup.data.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-04-24T20:35:48.629Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next 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[]`.
Applied to files:
packages/bootstrap-vue-next/src/types/ComponentEmits.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/types/ComponentProps.tspackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/components/** : Create and modify Vue components only under packages/bootstrap-vue-next/src/components/
Applied to files:
packages/bootstrap-vue-next/src/types/ComponentEmits.tspackages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-04-24T20:35:48.629Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/types/ComponentEmits.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-04-24T20:35:48.629Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/types/ComponentEmits.tspackages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/components/**/*.vue : Keep component-specific styles within their respective .vue single-file components
Applied to files:
packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to packages/bootstrap-vue-next/src/styles/styles.scss : Use packages/bootstrap-vue-next/src/styles/styles.scss as the main styles entry point
Applied to files:
packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vueapps/docs/src/data/components/app.data.ts
📚 Learning: 2025-05-23T23:58:07.165Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2701
File: apps/docs/src/docs/migration-guide.md:630-632
Timestamp: 2025-05-23T23:58:07.165Z
Learning: The `<NotYetImplemented/>` component in the bootstrap-vue-next documentation automatically renders text indicating "Not Yet Implemented", so additional explanatory text about features not being implemented is redundant when this component is used.
Applied to files:
packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue
📚 Learning: 2025-09-12T14:46:49.416Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T14:46:49.416Z
Learning: Applies to apps/docs/src/data/components/*.data.ts : When adding or modifying component props, events, or slots, ALWAYS update the corresponding .data.ts file in apps/docs/src/data/components/
Applied to files:
apps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tsapps/docs/src/data/components/listGroup.data.tspackages/bootstrap-vue-next/src/types/ComponentProps.tsapps/docs/src/data/components/accordion.data.ts
📚 Learning: 2025-05-26T17:28:35.902Z
Learnt from: VividLemon
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
apps/docs/src/data/components/orchestrator.data.tsapps/docs/src/data/components/app.data.tspackages/bootstrap-vue-next/src/composables/useBreadcrumb/index.ts
📚 Learning: 2025-05-28T07:57:19.915Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-05-28T07:01:55.095Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next 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.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vue
📚 Learning: 2025-08-20T14:05:32.416Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2732
File: packages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue:28-28
Timestamp: 2025-08-20T14:05:32.416Z
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.vueapps/docs/src/components/PageContentsItem.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vueapps/docs/src/components/PageContents.vue
📚 Learning: 2025-08-20T14:05:32.416Z
Learnt from: xvaara
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2732
File: packages/bootstrap-vue-next/src/components/BApp/BOrchestrator.vue:28-28
Timestamp: 2025-08-20T14:05:32.416Z
Learning: Vue 3 fully supports TypeScript annotations in template binding expressions when using `<script setup lang="ts">` or `<script lang="ts">`. Template ref callbacks with type annotations like `:ref="(ref: ComponentPublicInstance) => ..."` are valid and properly type-checked. Template expressions are fully type-checked by TypeScript in this setup.
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/BTable.vueapps/docs/src/components/PageContentsItem.vuepackages/bootstrap-vue-next/src/components/BTable/BTableLite.vueapps/docs/src/components/PageContents.vue
🧬 Code graph analysis (16)
packages/bootstrap-vue-next/src/composables/useModalManager.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
modalManagerKey(193-202)
packages/bootstrap-vue-next/src/composables/usePopover/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
orchestratorRegistryKey(210-214)
packages/bootstrap-vue-next/src/composables/useToast/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
orchestratorRegistryKey(210-214)
packages/bootstrap-vue-next/src/types/ComponentEmits.ts (1)
packages/bootstrap-vue-next/src/types/TableTypes.ts (1)
BTableSortBy(35-38)
packages/bootstrap-vue-next/src/composables/useModal/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
orchestratorRegistryKey(210-214)
packages/bootstrap-vue-next/src/composables/useTableLiteHelpers.ts (7)
packages/bootstrap-vue-next/src/types/TableTypes.ts (5)
TableFieldRaw(88-90)TableField(60-86)isTableItem(15-16)isTableField(92-93)TableItem(9-13)packages/bootstrap-vue-next/src/utils/tableUtils.ts (2)
StackedProps(64-67)getDataLabelAttr(69-70)packages/bootstrap-vue-next/src/utils/stringUtils.ts (1)
startCase(14-18)packages/bootstrap-vue-next/src/utils/keys.ts (1)
tableKeyboardNavigationKey(226-229)packages/bootstrap-vue-next/src/utils/event.ts (1)
stopEvent(91-104)packages/bootstrap-vue-next/src/utils/constants.ts (4)
CODE_DOWN(4-4)CODE_UP(14-14)CODE_HOME(8-8)CODE_END(5-5)packages/bootstrap-vue-next/src/utils/filterEvent.ts (1)
filterEvent(22-52)
packages/bootstrap-vue-next/src/utils/tableUtils.ts (1)
packages/bootstrap-vue-next/src/types/BreakpointProps.ts (1)
Breakpoint(3-3)
packages/bootstrap-vue-next/src/types/ComponentProps.ts (1)
packages/bootstrap-vue-next/src/types/TableTypes.ts (3)
TableFieldRaw(88-90)BTableSelectMode(24-24)BTableFilterFunction(33-33)
packages/bootstrap-vue-next/src/composables/useRegistry/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (4)
showHideRegistryKey(165-166)modalManagerKey(193-202)breadcrumbRegistryKey(188-191)rtlRegistryKey(182-185)
packages/bootstrap-vue-next/src/composables/useProvideDefaults/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
defaultsKey(204-205)
packages/bootstrap-vue-next/src/types/TableTypes.ts (1)
packages/bootstrap-vue-next/src/types/index.ts (1)
MaybePromise(91-91)
packages/bootstrap-vue-next/src/composables/orchestratorShared.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
orchestratorRegistryKey(210-214)
packages/bootstrap-vue-next/src/composables/useRtl.ts (2)
packages/bootstrap-vue-next/src/plugins/rtl/index.ts (1)
rtlPlugin(5-23)packages/bootstrap-vue-next/src/utils/keys.ts (1)
rtlRegistryKey(182-185)
packages/bootstrap-vue-next/src/composables/useTableHelpers.ts (7)
packages/bootstrap-vue-next/src/types/TableTypes.ts (14)
TableFieldRaw(88-90)BTableSortByComparerFunction(31-31)BTableSortBy(35-38)BTableFilterFunction(33-33)TableField(60-86)isTableField(92-93)TableFieldFormatter(52-52)isTableItem(15-16)TableItem(9-13)BTableSelectMode(24-24)BTableProvider(48-50)NoProviderTypes(98-98)BTableInitialSortDirection(29-29)BTableSortByOrder(22-22)packages/bootstrap-vue-next/src/utils/tableUtils.ts (2)
StackedProps(64-67)getDataLabelAttr(69-70)packages/bootstrap-vue-next/src/utils/stringUtils.ts (1)
startCase(14-18)packages/bootstrap-vue-next/src/utils/formatItem.ts (1)
formatItem(4-12)packages/bootstrap-vue-next/src/utils/keys.ts (1)
tableKeyboardNavigationKey(226-229)packages/bootstrap-vue-next/src/utils/debounce.ts (1)
useDebounceFn(102-108)packages/bootstrap-vue-next/src/utils/object.ts (1)
deepEqual(132-155)
packages/bootstrap-vue-next/src/composables/useShowHide.ts (2)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
showHideRegistryKey(165-166)packages/bootstrap-vue-next/src/utils/functions.ts (1)
noop(1-1)
packages/bootstrap-vue-next/src/composables/useBreadcrumb/index.ts (1)
packages/bootstrap-vue-next/src/utils/keys.ts (1)
breadcrumbRegistryKey(188-191)
🔇 Additional comments (27)
apps/docs/src/data/components/app.data.ts (1)
2-2: LGTM! Import path standardization.The change from absolute to relative import path is correct and aligns with the broader refactoring effort to standardize module resolution across the docs application.
apps/docs/src/data/components/orchestrator.data.ts (1)
2-2: LGTM! Consistent path update.The relative import path change matches the pattern established across other data component files.
apps/docs/src/data/components/accordion.data.ts (1)
8-8: LGTM! Path standardization applied consistently.The relative import path update is correct and maintains consistency with the other data component files in this directory.
apps/docs/src/data/components/listGroup.data.ts (1)
10-10: LGTM! Utility import path updated correctly.The change extends the import path standardization to utility imports, maintaining consistency across the codebase.
apps/docs/src/components/PageContentsItem.vue (1)
18-18: LGTM! Correct relative path for component location.The relative path depth (
../types) is appropriate for files in thecomponentsdirectory, differing from thedata/componentsfiles which use../../types.apps/docs/src/components/PageContents.vue (1)
13-13: LGTM! Matches sibling component pattern.The import path update is consistent with the adjacent
PageContentsItem.vuefile, completing the standardization across components in this directory.packages/bootstrap-vue-next/src/utils/functions.ts (1)
1-1: Centralizednoophelper is fineExporting
noophere is simple, typed correctly by inference, and suitable as a shared internal utility.packages/bootstrap-vue-next/src/utils/debounce.ts (1)
10-10: Using sharednoopkeeps debounce behavior unchanged and DRYImporting
noopfrom./functionsmaintains existing semantics forlastRejectorwhile avoiding duplicate utility definitions.packages/bootstrap-vue-next/src/composables/useToast/index.ts (1)
27-27: LGTM! Explicit null default improves type safety.The injection now uses an explicit
nulldefault, making the optional injection behavior clear in the type system. The existing error-throwing logic continues to handle the case where no registry is provided.packages/bootstrap-vue-next/src/composables/orchestratorShared.ts (1)
135-135: LGTM! Consistent with the injection pattern updates.The explicit
nulldefault aligns with the broader refactoring across composables while preserving the existing fallback behavior of creating a new registry when none is provided.packages/bootstrap-vue-next/src/components/BNavbar/BNavbarToggle.vue (1)
36-36: LGTM! Explicit null default with proper guards.The injection now uses an explicit
nulldefault. The existing logic at lines 39 and 45 already guards against missingshowHideData, so behavior is preserved.packages/bootstrap-vue-next/src/composables/useRtl.ts (1)
5-5: LGTM! Explicit null default with optional chaining.The injection now uses an explicit
nulldefault, with appropriate optional chaining added at lines 10 and 19 to safely access the plugin's properties.packages/bootstrap-vue-next/src/composables/usePopover/index.ts (1)
27-27: LGTM! Consistent injection pattern.The explicit
nulldefault improves type safety while the error-throwing logic at lines 28-32 preserves the existing behavior when no registry is provided.packages/bootstrap-vue-next/src/composables/useModalManager.ts (1)
16-16: LGTM! Explicit null default with proper guards.The injection now uses an explicit
nulldefault. The code correctly uses optional chaining at lines 22-23, 41, and 47 to guard against missing plugin.packages/bootstrap-vue-next/src/types/ComponentEmits.ts (2)
48-48: LGTM! Readonly constraints improve type safety.The
filteredandchangeevents now emitreadonly Items[]instead ofItems[], preventing accidental mutations of the emitted arrays. This is a good practice that aligns with the broader type system improvements in the PR.Also applies to: 52-52
114-114: LGTM! Readonly constraint for tag-state event.The
tag-stateevent now usesreadonly string[][], ensuring the emitted array cannot be accidentally mutated by event handlers.packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue (2)
202-202: LGTM! Improved encapsulation with setter function.The destructured
setLocalNoAnimationreplaces direct mutation of thelocalNoAnimationref, providing better encapsulation of the show/hide state management.
298-317: LGTM! Proper usage of the setter function.The code now correctly calls
setLocalNoAnimation(true)at lines 302 and 310 instead of directly mutating the ref, aligning with the updateduseShowHideAPI.packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue (9)
219-248: Imports and composable wiring look correct and minimal
computed,btableSimpleProps,getTableFieldHeadLabel, and the newuseTableLiteHelpersimports are all used and keep responsibilities in shared utilities/composables. No issues spotted with paths, usage, or apparent tree‑shaking behavior.
298-301: useTableDetails integration is straightforward and reactivePassing
itemsandprimaryKeyas functions intouseTableDetailsensures the composable tracks prop changes without extra watchers. The returnedgetDetailsValue/toggleRowDetailsare used consistently in the template for details visibility and toggling.
302-325: Keyboard navigation composable is cleanly wired to emits
useTableKeyboardNavigationis configured with the tableidand reactiveitems, and the callbacks re‑emithead-clicked,row-clicked, androw-middle-clickedwith arguments that match the expected emit signatures. Pointer events in the template still emit directly, so this composable nicely unifies keyboard behavior without changing existing mouse semantics.
326-337: Fields/headers mapping via useTableFieldsMapper looks consistentDelegating
computedFields,computedFieldsTotal, andshowComputedHeaderstouseTableFieldsMapperwithfields,items, andstackedPropspassed as functions keeps things reactive and aligns with the stacked/labelStacked behavior. Usage in the template (thead, tbody, tfoot) matches these outputs.
348-353: computedTableClasses correctly composes table classesThe computed wraps
props.tableClassand conditionally adds thealign-*class only whenprops.alignis defined, which avoids spurious classes and keeps alignment styling centralized for BTableSimple viacomputedSimpleProps.
379-385: getRowClasses now cleanly delegates to tbodyTrClassThis helper simply forwards
(item, type)toprops.tbodyTrClasswhen provided (or returnsnullotherwise), which reduces duplication and keeps row-class customization in a single prop. Variant styling is handled separately on theBTrvia thevariantprop, so this change should not drop visual variants.
386-389: itemAttributes correctly handles functional vs static tdAttrThe helper computes
valwithget(item, fieldKey)and invokesattr(val, fieldKey, item)only whenattris a function, otherwise it returns the staticattrvalue. This matches the usualtdAttrcontract and keepsv-bind="itemAttributes(...)"flexible for both function and object forms.
390-395: callThAttr unifies header TH attribute handling
callThAttrmirrorsitemAttributesbut forthAttr, passing(val, fieldKey, item, type)whenthAttris a function or returning it as-is otherwise. Using this for both top and bottom headers (type='top' | 'bottom'etc.) keeps the API consistent and avoids duplication.
414-417: footerProps computed keeps head/foot styling in syncComputing
footerPropswithvariant: props.footVariant ?? props.headVariantandclass: props.tfootClass ?? props.theadClassmatches expectations that the foot defaults mirror the head unless explicitly overridden. Binding this into bothBTfootbranches is a nice consolidation.
| } else if (shiftClicked) { | ||
| const lastSelectedItem = [...selectedItemsToSet.value].pop() | ||
| const lastSelectedIndex = allItemsResolved.value.findIndex((i) => i === lastSelectedItem) | ||
| const selectStartIndex = Math.min(lastSelectedIndex, index) | ||
| const selectEndIndex = Math.max(lastSelectedIndex, index) | ||
| const items = allItemsResolved.value.slice(selectStartIndex, selectEndIndex + 1) | ||
| set(items) | ||
| // If nothing is being held, then we just behave like it's single mode | ||
| } else { | ||
| set([row]) | ||
| } |
There was a problem hiding this comment.
Handle range selection without an anchor.
In range mode, the first Shift+click (or any case where the last anchor is missing) now slices with -1, producing an empty selection instead of the row the user picked. Guard the -1 case and fall back to selecting the clicked row.
- } else if (shiftClicked) {
- const lastSelectedItem = [...selectedItemsToSet.value].pop()
- const lastSelectedIndex = allItemsResolved.value.findIndex((i) => i === lastSelectedItem)
+ } else if (shiftClicked) {
+ const lastSelectedItem = [...selectedItemsToSet.value].pop()
+ const lastSelectedIndex =
+ lastSelectedItem === undefined
+ ? -1
+ : allItemsResolved.value.findIndex((i) => i === lastSelectedItem)
+ if (lastSelectedIndex < 0) {
+ set([row])
+ return
+ }
const selectStartIndex = Math.min(lastSelectedIndex, index)
const selectEndIndex = Math.max(lastSelectedIndex, index)
const items = allItemsResolved.value.slice(selectStartIndex, selectEndIndex + 1)
set(items)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (shiftClicked) { | |
| const lastSelectedItem = [...selectedItemsToSet.value].pop() | |
| const lastSelectedIndex = allItemsResolved.value.findIndex((i) => i === lastSelectedItem) | |
| const selectStartIndex = Math.min(lastSelectedIndex, index) | |
| const selectEndIndex = Math.max(lastSelectedIndex, index) | |
| const items = allItemsResolved.value.slice(selectStartIndex, selectEndIndex + 1) | |
| set(items) | |
| // If nothing is being held, then we just behave like it's single mode | |
| } else { | |
| set([row]) | |
| } | |
| } else if (shiftClicked) { | |
| const lastSelectedItem = [...selectedItemsToSet.value].pop() | |
| const lastSelectedIndex = | |
| lastSelectedItem === undefined | |
| ? -1 | |
| : allItemsResolved.value.findIndex((i) => i === lastSelectedItem) | |
| if (lastSelectedIndex < 0) { | |
| set([row]) | |
| return | |
| } | |
| const selectStartIndex = Math.min(lastSelectedIndex, index) | |
| const selectEndIndex = Math.max(lastSelectedIndex, index) | |
| const items = allItemsResolved.value.slice(selectStartIndex, selectEndIndex + 1) | |
| set(items) | |
| // If nothing is being held, then we just behave like it's single mode | |
| } else { | |
| set([row]) | |
| } |
🤖 Prompt for AI Agents
In packages/bootstrap-vue-next/src/composables/useTableHelpers.ts around lines
434 to 444, the range-selection path uses lastSelectedIndex which can be -1 when
there is no anchor, causing slice to return an incorrect/empty range; change the
logic to detect lastSelectedIndex === -1 and in that case fall back to selecting
only the clicked row (i.e., call set([row]) or treat lastSelectedIndex as index
before computing start/end), otherwise compute selectStartIndex/selectEndIndex
and call set with the sliced range.
| items: fieldsValue.map((f) => { | ||
| if (isTableField(f)) { | ||
| const label = f.label ?? startCase(f.key as string) | ||
| return { | ||
| ...(f as TableField), | ||
| tdAttr: {...getDataLabelAttr(stacked, label), ...f.tdAttr}, | ||
| } | ||
| } | ||
| const label = startCase(f as string) | ||
| return { | ||
| key: f as string, | ||
| label, | ||
| tdAttr: getDataLabelAttr(stacked, label), | ||
| } | ||
| }), |
There was a problem hiding this comment.
Don't clobber user-provided tdAttr functions.
Spreading into tdAttr assumes it is an object; when callers supply a function we now replace it with {}, so their dynamic cell attributes never run. Please preserve functions and only merge data-labels when the original value is static.
- return {
- ...(f as TableField),
- tdAttr: {...getDataLabelAttr(stacked, label), ...f.tdAttr},
- }
+ const dataLabelAttr = getDataLabelAttr(stacked, label)
+ if (typeof f.tdAttr === 'function') {
+ const originalTdAttr = f.tdAttr
+ return {
+ ...(f as TableField),
+ tdAttr: (value, key, item) => ({
+ ...dataLabelAttr,
+ ...originalTdAttr(value, key, item),
+ }),
+ }
+ }
+ return {
+ ...(f as TableField),
+ tdAttr: dataLabelAttr
+ ? {...dataLabelAttr, ...(f.tdAttr ?? {})}
+ : f.tdAttr,
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| items: fieldsValue.map((f) => { | |
| if (isTableField(f)) { | |
| const label = f.label ?? startCase(f.key as string) | |
| return { | |
| ...(f as TableField), | |
| tdAttr: {...getDataLabelAttr(stacked, label), ...f.tdAttr}, | |
| } | |
| } | |
| const label = startCase(f as string) | |
| return { | |
| key: f as string, | |
| label, | |
| tdAttr: getDataLabelAttr(stacked, label), | |
| } | |
| }), | |
| items: fieldsValue.map((f) => { | |
| if (isTableField(f)) { | |
| const label = f.label ?? startCase(f.key as string) | |
| const dataLabelAttr = getDataLabelAttr(stacked, label) | |
| if (typeof f.tdAttr === 'function') { | |
| const originalTdAttr = f.tdAttr | |
| return { | |
| ...(f as TableField), | |
| tdAttr: (value, key, item) => ({ | |
| ...dataLabelAttr, | |
| ...originalTdAttr(value, key, item), | |
| }), | |
| } | |
| } | |
| return { | |
| ...(f as TableField), | |
| tdAttr: dataLabelAttr | |
| ? {...dataLabelAttr, ...(f.tdAttr ?? {})} | |
| : f.tdAttr, | |
| } | |
| } | |
| const label = startCase(f as string) | |
| return { | |
| key: f as string, | |
| label, | |
| tdAttr: getDataLabelAttr(stacked, label), | |
| } | |
| }), |
🤖 Prompt for AI Agents
packages/bootstrap-vue-next/src/composables/useTableLiteHelpers.ts around lines
60 to 74: the current implementation spreads getDataLabelAttr into f.tdAttr
assuming it's an object, which overwrites user-provided functions; change the
merge to detect if f.tdAttr is a function and, if so, replace tdAttr with a new
function that calls the original function to get its attrs and then merges the
data-label attrs into that result; if f.tdAttr is an object merge as currently
done; for non-table-field branches leave tdAttr as getDataLabelAttr (or wrap
similarly if you want consistent behavior) so dynamic tdAttr functions are
preserved and static objects still receive the data-label attribute.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the BTable component by extracting complex logic into reusable composables, improving code organization, maintainability, and testability. The refactoring introduces new helper composables for table field mapping, row details management, keyboard navigation, sorting, filtering, selection, and provider support.
Key changes:
- Extracted table logic into two new composable files:
useTableHelpers.ts(for BTable) anduseTableLiteHelpers.ts(for BTableLite) - Improved type safety with readonly array types throughout the codebase
- Fixed inject calls to use
nullas default instead ofundefinedfor better type inference - Added utility exports and types to support the composable architecture
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
packages/bootstrap-vue-next/src/composables/useTableHelpers.ts |
New composables for table mapper, keyboard navigation, selection, provider, and sorting logic |
packages/bootstrap-vue-next/src/composables/useTableLiteHelpers.ts |
New composables for fields mapping, row details, and keyboard navigation for lite tables |
packages/bootstrap-vue-next/src/composables/useShowHide.ts |
Refactored transition callbacks to arrays and added setter functions for readonly state |
packages/bootstrap-vue-next/src/components/BTable/BTable.vue |
Refactored to use new composables, significantly reducing component complexity |
packages/bootstrap-vue-next/src/components/BTable/BTableLite.vue |
Refactored to use new lite helpers composables |
packages/bootstrap-vue-next/src/types/TableTypes.ts |
Added new types and converted arrays to readonly for immutability |
packages/bootstrap-vue-next/src/types/ComponentProps.ts |
Converted array props to readonly for better immutability |
packages/bootstrap-vue-next/src/types/ComponentEmits.ts |
Converted array emit values to readonly |
packages/bootstrap-vue-next/src/utils/functions.ts |
New utility file with noop function export |
packages/bootstrap-vue-next/src/utils/tableUtils.ts |
Added StackedProps type for better type reuse |
| Multiple inject usage files | Changed inject default from undefined to null for better type inference |
| Documentation data files | Fixed import paths from src/types to relative paths |
| export const useTableDetails = ({ | ||
| items, | ||
| primaryKey, | ||
| }: { | ||
| items: MaybeRefOrGetter<readonly unknown[]> | ||
| primaryKey: MaybeRefOrGetter<string | undefined> | ||
| }) => { |
There was a problem hiding this comment.
The useTableDetails composable lacks documentation. Add a JSDoc comment explaining its purpose (managing row details visibility with Map/WeakMap), parameters, and return values.
| export const useTableMapper = <Items>({ | ||
| fields, | ||
| items, | ||
| stackedProps, | ||
| provider, | ||
| events, | ||
| pagination, | ||
| }: { | ||
| items: MaybeRefOrGetter<readonly Items[]> | ||
| fields: MaybeRefOrGetter<readonly TableFieldRaw<Items>[]> | ||
| stackedProps: { | ||
| stacked: MaybeRefOrGetter<StackedProps['stacked']> | ||
| labelStacked: MaybeRefOrGetter<StackedProps['labelStacked']> | ||
| } | ||
| pagination: { | ||
| perPage: MaybeRefOrGetter<number> | ||
| currentPage: MaybeRefOrGetter<number> | ||
| sort: { | ||
| isSortable: MaybeRefOrGetter<boolean> | ||
| sortCompare: MaybeRef<BTableSortByComparerFunction<Items> | undefined> | ||
| by: MaybeRefOrGetter<readonly BTableSortBy[] | undefined> | ||
| noLocalSorting: MaybeRefOrGetter<boolean> | ||
| } | ||
| filter: { | ||
| filterFunction: MaybeRef<BTableFilterFunction<Items> | undefined> | ||
| filter: MaybeRefOrGetter<string | undefined> | ||
| filterable: MaybeRefOrGetter<readonly string[] | undefined> | ||
| } | ||
| } | ||
| provider: { | ||
| noProviderSorting: MaybeRefOrGetter<boolean> | ||
| noProviderPaging: MaybeRefOrGetter<boolean> | ||
| noProviderFiltering: MaybeRefOrGetter<boolean> | ||
| usesProvider: MaybeRefOrGetter<boolean> | ||
| items: MaybeRefOrGetter<readonly Items[]> | ||
| } | ||
| events: { | ||
| onChange: (items: readonly Items[]) => void | ||
| } | ||
| }) => { |
There was a problem hiding this comment.
The useTableMapper composable lacks documentation. Add a JSDoc comment explaining its purpose (mapping and transforming table data, filtering, sorting), parameters, and return values. This is a complex composable that would benefit from clear documentation.
| export const useTableKeyboardNavigationInjector = ({ | ||
| isSortable, | ||
| selectable, | ||
| noSelectOnClick, | ||
| }: { | ||
| selectable: MaybeRefOrGetter<boolean> | ||
| noSelectOnClick: MaybeRefOrGetter<boolean> | ||
| isSortable: MaybeRefOrGetter<boolean> | ||
| }) => { |
There was a problem hiding this comment.
The useTableKeyboardNavigationInjector composable lacks documentation. Add a JSDoc comment explaining its purpose (providing keyboard navigation state to child components via Vue's provide/inject).
| export const useTableSelectedItems = <Item>({ | ||
| selectable, | ||
| selectMode, | ||
| selectedItems, | ||
| events, | ||
| primaryKey, | ||
| allItems, | ||
| }: { | ||
| allItems: MaybeRefOrGetter<readonly Item[]> | ||
| selectable: MaybeRefOrGetter<boolean> | ||
| selectMode: MaybeRefOrGetter<BTableSelectMode> | ||
| primaryKey: MaybeRefOrGetter<string | undefined> | ||
| selectedItems: Ref<readonly Item[]> | ||
| events: { | ||
| onRowSelected: (item: Item) => void | ||
| onRowUnselected: (item: Item) => void | ||
| } | ||
| }) => { |
There was a problem hiding this comment.
The useTableSelectedItems composable lacks documentation. Add a JSDoc comment explaining its purpose (managing table row selection with single/multi/range modes), parameters, and return values.
| export const useTableProvider = <Item>({ | ||
| provider, | ||
| busy, | ||
| currentPage, | ||
| debounce, | ||
| perPage, | ||
| noProvider, | ||
| noProviderFiltering, | ||
| noProviderPaging, | ||
| noProviderSorting, | ||
| filter, | ||
| sortBy, | ||
| events, | ||
| }: { | ||
| sortBy: MaybeRefOrGetter<readonly BTableSortBy[] | undefined> | ||
| currentPage: MaybeRefOrGetter<number> | ||
| perPage: MaybeRefOrGetter<number> | ||
| filter: MaybeRefOrGetter<string | undefined> | ||
| busy: Ref<boolean> | ||
| provider: MaybeRefOrGetter<BTableProvider<Item> | undefined> | ||
| debounce: { | ||
| wait: MaybeRefOrGetter<number> | ||
| maxWait: MaybeRefOrGetter<number> | ||
| } | ||
| noProvider: MaybeRefOrGetter<readonly NoProviderTypes[] | undefined> | ||
| noProviderPaging: MaybeRefOrGetter<boolean> | ||
| noProviderSorting: MaybeRefOrGetter<boolean> | ||
| noProviderFiltering: MaybeRefOrGetter<boolean> | ||
| events: { | ||
| onFiltered: () => void | ||
| } | ||
| }) => { |
There was a problem hiding this comment.
The useTableProvider composable lacks documentation. Add a JSDoc comment explaining its purpose (managing asynchronous data provider with debouncing and abort support), parameters, and return values.
| @@ -0,0 +1 @@ | |||
| export const noop = () => {} | |||
There was a problem hiding this comment.
The noop function is exported but lacks documentation. Consider adding a JSDoc comment explaining its purpose: /** Empty function that does nothing. Useful as a default callback or placeholder. */
| export const useTableFieldsMapper = <Item>({ | ||
| fields, | ||
| items, | ||
| stackedProps, | ||
| }: { | ||
| fields: MaybeRefOrGetter<readonly TableFieldRaw<Item>[]> | ||
| items: MaybeRefOrGetter<readonly Item[]> | ||
| stackedProps: { | ||
| stacked: MaybeRefOrGetter<StackedProps['stacked']> | ||
| labelStacked: MaybeRefOrGetter<StackedProps['labelStacked']> | ||
| } | ||
| }) => { |
There was a problem hiding this comment.
The useTableFieldsMapper composable lacks documentation. Add a JSDoc comment explaining its purpose, parameters, and return values to help developers understand when and how to use it.
| export const useTableKeyboardNavigation = <Items>( | ||
| { | ||
| items, | ||
| id, | ||
| }: { | ||
| items: MaybeRefOrGetter<readonly Items[]> | ||
| id: MaybeRefOrGetter<string> | ||
| }, | ||
| events: { | ||
| onHeadClicked: ( | ||
| fieldKey: string, | ||
| field: TableField<Items>, | ||
| event: MouseEvent, | ||
| isFooter: boolean | ||
| ) => void | ||
| onRowClicked: (item: Items, itemIndex: number, event: MouseEvent) => void | ||
| onRowMiddleClicked: (item: Items, itemIndex: number, event: MouseEvent) => void | ||
| } | ||
| ) => { |
There was a problem hiding this comment.
The useTableKeyboardNavigation composable lacks documentation. Add a JSDoc comment explaining its purpose (keyboard navigation support for tables), parameters, and return values.
| const onLeave: TransitionProps['onLeave'] = [ | ||
| () => { | ||
| isVisible.value = false | ||
| }, | ||
| ...[options.transitionProps?.onLeave ?? noop, props.transitionProps?.onLeave ?? noop].flat(), | ||
| ] |
There was a problem hiding this comment.
Same issue - Vue's Transition expects a single function, not an array. This needs to be wrapped in a single function that invokes the array elements.
| const onAfterLeave: TransitionProps['onAfterLeave'] = [ | ||
| () => { | ||
| emit('hidden', buildTriggerableEvent('hidden', {trigger: leaveTrigger, cancelable: false})) | ||
| }, | ||
| ...[ | ||
| options.transitionProps?.onAfterLeave ?? noop, | ||
| props.transitionProps?.onAfterLeave ?? noop, | ||
| ].flat(), | ||
| () => { | ||
| isLeaving.value = false | ||
| isActive.value = false | ||
| if (localNoAnimation.value) { | ||
| requestAnimationFrame(() => { | ||
| localNoAnimation.value = false | ||
| }) | ||
| } | ||
| requestAnimationFrame(() => { | ||
| if (!localTemporaryHide.value) renderRef.value = false | ||
| }) | ||
| }) | ||
| if (!_resolveOnHide) { | ||
| _Resolve?.(true) | ||
| _Resolve?.(leaveTrigger || '') | ||
| _Promise = undefined | ||
| _Resolve = undefined | ||
| } | ||
| } | ||
| const onBeforeLeave = (el: Element) => { | ||
| if (!isLeaving.value) isLeaving.value = true | ||
| options.transitionProps?.onBeforeLeave?.(el) | ||
| props.transitionProps?.onBeforeLeave?.(el) | ||
| trapActive.value = false | ||
| } | ||
| const onLeave = (el: Element) => { | ||
| isVisible.value = false | ||
| options.transitionProps?.onLeave?.(el) | ||
| props.transitionProps?.onLeave?.(el) | ||
| } | ||
| const onAfterLeave = (el: Element) => { | ||
| emit('hidden', buildTriggerableEvent('hidden', {trigger: leaveTrigger, cancelable: false})) | ||
| options.transitionProps?.onAfterLeave?.(el) | ||
| props.transitionProps?.onAfterLeave?.(el) | ||
| isLeaving.value = false | ||
| isActive.value = false | ||
| if (localNoAnimation.value) { | ||
| requestAnimationFrame(() => { | ||
| localNoAnimation.value = false | ||
| }) | ||
| } | ||
| requestAnimationFrame(() => { | ||
| if (!localTemporaryHide.value) renderRef.value = false | ||
| }) | ||
| _Resolve?.(leaveTrigger || '') | ||
| _Promise = undefined | ||
| _Resolve = undefined | ||
| leaveTrigger = undefined | ||
| } | ||
| leaveTrigger = undefined | ||
| }, | ||
| ] |
There was a problem hiding this comment.
Same issue - Vue's Transition expects a single function, not an array. This needs to be wrapped in a single function that invokes the array elements.
1 participant
Describe the PR
A clear and concise description of what the pull request does.
Small replication
A small replication or video walkthrough can help demonstrate the changes made. This is optional, but can help observe the intended changes. A mentioned issue that contains a replication also works.
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
Improvements