fix(BTable): make sort icons background images to avoid wrapping #2924
Conversation
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a boolean prop Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Consumer (app/demo)
participant BTable as BTable component
participant Computed as computedFields
participant DOM as Header cell (th)
participant SCSS as _table.scss
note over Consumer,BTable: Render request with props (fields, items, sortIconLeft)
Consumer->>BTable: mount/render(props)
BTable->>Computed: build fields (add thClass when sortIconLeft)
Computed-->>BTable: fields with thClass
BTable->>DOM: render header cells with aria-sort + thClass
SCSS->>DOM: apply background-image based on [aria-sort] and thClass
DOM-->>Consumer: header visuals (icon left or right via CSS)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
commit: |
There was a problem hiding this comment.
Pull request overview
This PR converts BTable sort icons from inline SVG elements to CSS background images to prevent wrapping issues, bringing the implementation closer to Bootstrap Vue (BSV) compatibility. It also implements the sort-icon-left prop to allow positioning icons on the left side of header cells.
- Replaces inline SortIcon component with CSS background image approach
- Adds
sortIconLeftprop for flexible icon positioning - Updates tests to verify CSS-based icon implementation
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/bootstrap-vue-next/src/types/ComponentProps.ts |
Adds sortIconLeft boolean prop to BTableProps interface |
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts |
Updates tests to verify CSS background image icons and sortIconLeft functionality |
packages/bootstrap-vue-next/src/components/BTable/_table.scss |
Adds CSS for background image sort icons with positioning logic (contains critical bug - missing str-replace function) |
packages/bootstrap-vue-next/src/components/BTable/BTable.vue |
Removes SortIcon component usage, applies b-table-sort-icon-left class conditionally (has bug - applies to all fields) |
apps/docs/src/docs/migration-guide.md |
Removes placeholder comment about BTable not being documented |
apps/docs/src/docs/components/table.md |
Adds documentation for sort icon positioning feature |
apps/docs/src/docs/components/demo/TableSortIcons.vue |
Adds demo showing default and left-aligned sort icons |
apps/docs/src/data/components/table.data.ts |
Documents the new sortIconLeft prop |
![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 (1)
packages/bootstrap-vue-next/src/components/BTable/BTable.vue (1)
146-151: sortIconLeft integration is solid, but noSortableIcon is now ignoredThe new bits here look good:
sortIconLefthas a sane default offalseinwithDefaults.computedFieldsextends each field withthAttr.aria-sortand athClassthat conditionally adds'b-table-sort-icon-left'based onprops.sortIconLeft, which lines up with the SCSS selectors and tests.However, the
noSortableIconprop is no longer connected to the rendering pipeline:aria-sortis always set for sortable headers, and SCSS now always renders background icons for[aria-sort]cells. SincetableClassesdoesn’t add any class fornoSortableIcon, settingno-sort-iconon<BTable>no longer hides icons.To restore that behavior while preserving accessibility:
- Add a suppressor class at the table level:
const tableClasses = computed(() => ({ 'b-table-busy': busyModel.value, 'b-table-selectable': props.selectable, 'user-select-none': props.selectable && isSelecting.value, 'b-table-fixed': props.fixed, 'b-table-no-border-collapse': props.noBorderCollapse, + 'b-table-no-sort-icon': props.noSortableIcon, }))
- In
_table.scss, scope the sort‑icon background rules under.table.b-table:not(.b-table-no-sort-icon)as suggested in the stylesheet review, sono-sort-icononly affects the visual icons, notaria-sortor click behavior.This keeps the new implementation and aria semantics intact while avoiding a regression for existing users of
noSortableIcon.Also applies to: 371-385, 387-393
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts (1)
1070-1121: New sort icon tests align with aria-sort & sortIconLeft behaviorThe added tests:
- Validate that sortable headers expose
aria-sortwith one ofnone | ascending | descending.- Confirm
b-table-sort-icon-leftis applied whensortIconLeftis true and absent when false, and that the legacyb-table-sort-icon-inlineclass is not used.These assertions match the new CSS‑background implementation.
It would be worth adding a small test once
noSortableIconis wired back in (e.g., checking that the root<table>gains ab-table-no-sort-iconclass whenno-sort-iconis set), but that can follow the implementation fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/docs/src/data/components/table.data.ts(1 hunks)apps/docs/src/docs/components/demo/TableSortIcons.vue(1 hunks)apps/docs/src/docs/components/table.md(4 hunks)apps/docs/src/docs/migration-guide.md(0 hunks)packages/bootstrap-vue-next/src/components/BTable/BTable.vue(2 hunks)packages/bootstrap-vue-next/src/components/BTable/_table.scss(1 hunks)packages/bootstrap-vue-next/src/components/BTable/table.spec.ts(2 hunks)packages/bootstrap-vue-next/src/types/ComponentProps.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/src/docs/migration-guide.md
🧰 Additional context used
📓 Path-based instructions (4)
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/table.data.ts
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/BTable/table.spec.tspackages/bootstrap-vue-next/src/components/BTable/_table.scsspackages/bootstrap-vue-next/src/components/BTable/BTable.vue
**/*.spec.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write tests following existing patterns in *.spec.ts files
Files:
packages/bootstrap-vue-next/src/components/BTable/table.spec.ts
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/BTable/BTable.vue
🧠 Learnings (17)
📓 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.
📚 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:
apps/docs/src/data/components/table.data.tsapps/docs/src/docs/components/demo/TableSortIcons.vuepackages/bootstrap-vue-next/src/components/BTable/table.spec.tsapps/docs/src/docs/components/table.mdpackages/bootstrap-vue-next/src/components/BTable/_table.scsspackages/bootstrap-vue-next/src/components/BTable/BTable.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/table.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/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/types/ComponentProps.tsapps/docs/src/docs/components/demo/TableSortIcons.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.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/types/ComponentProps.tspackages/bootstrap-vue-next/src/components/BTable/BTable.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/types/ComponentProps.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 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/ComponentProps.tsapps/docs/src/docs/components/demo/TableSortIcons.vue
📚 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/types/ComponentProps.tspackages/bootstrap-vue-next/src/components/BTable/table.spec.tsapps/docs/src/docs/components/table.mdpackages/bootstrap-vue-next/src/components/BTable/_table.scsspackages/bootstrap-vue-next/src/components/BTable/BTable.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/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/types/ComponentProps.tsapps/docs/src/docs/components/demo/TableSortIcons.vuepackages/bootstrap-vue-next/src/components/BTable/table.spec.tspackages/bootstrap-vue-next/src/components/BTable/_table.scsspackages/bootstrap-vue-next/src/components/BTable/BTable.vue
📚 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:
apps/docs/src/docs/components/demo/TableSortIcons.vue
📚 Learning: 2025-10-21T01:11:59.901Z
Learnt from: dwgray
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2861
File: apps/docs/src/docs/components/demo/CardKitchenSink.vue:28-31
Timestamp: 2025-10-21T01:11:59.901Z
Learning: In the bootstrap-vue-next docs codebase, the apps/docs/.vitepress/plugins/demo-container.ts plugin automatically generates kebab-case IDs from demo component filenames (e.g., CardKitchenSink.vue becomes id="card-kitchen-sink") and injects them into the HighlightCard wrapper elements. Therefore, demo components can use href="#kebab-case-filename" anchors without explicitly declaring the id attribute in the component template, as the id is generated at build time.
Applied to files:
apps/docs/src/docs/components/demo/TableSortIcons.vue
📚 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:
apps/docs/src/docs/components/demo/TableSortIcons.vuepackages/bootstrap-vue-next/src/components/BTable/BTable.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:
apps/docs/src/docs/components/demo/TableSortIcons.vuepackages/bootstrap-vue-next/src/components/BTable/_table.scsspackages/bootstrap-vue-next/src/components/BTable/BTable.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:
apps/docs/src/docs/components/demo/TableSortIcons.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:
apps/docs/src/docs/components/demo/TableSortIcons.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 **/*.spec.ts : Write tests following existing patterns in *.spec.ts files
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/table.spec.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/BTable/_table.scss
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: build
🔇 Additional comments (4)
packages/bootstrap-vue-next/src/types/ComponentProps.ts (1)
1162-1206: BTableProps: sortIconLeft prop wiring looks correctAdding
sortIconLeft?: booleanintoBTableProps<Items>is consistent with how the prop is used inBTable.vue, SCSS, tests, and docs. No typing or API-shape issues here.apps/docs/src/docs/components/demo/TableSortIcons.vue (1)
1-41: TableSortIcons demo correctly showcases sort-icon-leftThe demo cleanly contrasts default vs
sort-icon-leftbehavior, and theTableItem<Person>/TableFieldRaw<Person>typings align with existing table demos and the exported types.apps/docs/src/data/components/table.data.ts (1)
595-767: BTable docs metadata for sortIconLeft matches runtime/APIThe
sortIconLeftprop entry (type: 'boolean', default: false) and description accurately reflect the component prop and SCSS behavior, and it’s placed alongside the other sort-related props.apps/docs/src/docs/components/table.md (1)
777-786: Sort Icon Positioning docs match the new implementationThe new section correctly explains that sort icons are CSS background images anchored to the right by default, and that
sort-icon-leftswitches them to the left edge. The linkedTableSortIcons.vuedemo reinforces the behavior and aligns with the BTable prop and SCSS changes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/docs/src/docs/components/table.md (1)
885-891: ✓ Provider contextsortBydescription now matches its type signature.The updated description at line 890 now correctly reflects that
sortByis an array of{ key, order }entries (orundefined), aligning with the actual typeBTableSortBy<T>[] | undefined. This directly addresses the discrepancy flagged in past reviews.
🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/components/BTable/_table.scss (1)
228-245: Minor: Remove redundant:not(.b-table-no-sort-icon)selector in nested rule.Since the outer
.table.b-table:not(.b-table-no-sort-icon)already gates the icon styles, the nested&.table-sm:not(.b-table-no-sort-icon)on line 228 repeats the:not()check unnecessarily. Simplify to&.table-sm:- &.table-sm:not(.b-table-no-sort-icon) { + &.table-sm {This is a minor code-quality refinement with no functional impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/docs/src/docs/components/table.md(4 hunks)packages/bootstrap-vue-next/src/components/BTable/_table.scss(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/BTable/_table.scss
🧠 Learnings (7)
📓 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.
📚 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:
apps/docs/src/docs/components/table.mdpackages/bootstrap-vue-next/src/components/BTable/_table.scss
📚 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:
apps/docs/src/docs/components/table.md
📚 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:
apps/docs/src/docs/components/table.mdpackages/bootstrap-vue-next/src/components/BTable/_table.scss
📚 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/BTable/_table.scss
📚 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/BTable/_table.scss
📚 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/components/BTable/_table.scss
⏰ 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)
packages/bootstrap-vue-next/src/components/BTable/_table.scss (2)
155-173: ✓ Helper functions correctly defined and address past review concerns.The
str-replaceandbv-escape-svgfunctions are now properly implemented. The SVG escaping (encoding<,>,#as%3c,%3e,%23) is the standard approach for embedding SVG in CSS data URLs and will work across browsers.
190-226: ✓ Icon visibility gating correctly resolves past concerns aboutnoSortableIconsupport.The selector
.table.b-table:not(.b-table-no-sort-icon)successfully gates the sort icon CSS, allowing thenoSortableIconprop to work as expected. Icons will only render when theb-table-no-sort-iconclass is absent.apps/docs/src/docs/components/table.md (2)
777-785: ✓ New "Sort Icon Positioning" section is clear and helpful.This new documentation section effectively explains the default right-aligned icon behavior, the
sort-icon-leftprop, and provides a reference to the demo. The note about advanced customization via scoped slots is a good cross-reference.
903-903: ✓ Debouncing and AbortSignal examples are accurate and well-illustrated.Both the concise debouncing example (line 903) and the Promise-based AbortSignal cleanup pattern (lines 930–936) correctly demonstrate the updated API, with proper
clearTimeoutand error handling on abort.Also applies to: 930-936
|
@VividLemon This is ready to go - I’m not 100% I like the icon positioning on the far right - but that’s what BSV did, and it wasn’t trivial to give the option to continue to inline them (unless I missed something in CSS’s abilities). I think this is the better solution overall... |
- Add dark mode SVG icon variants with light colors (#dee2e6) - Add CSS rules for prefers-color-scheme: dark media query - Add CSS rules for [data-bs-theme="dark"] selector - Replace currentColor with explicit colors for both light and dark modes - Fixes regression from #2924 and #2933 Co-authored-by: dwgray <[email protected]>
2 participants
Describe the PR
Fixes #2923
Move sort icons to be background images as they were in BSV. Also implemented
sort-icon-leftwhile in the code.This brings us closer to BSV compatibility. It does change the look of current BSVN tables, as the old icons were inline with the text.
Small replication
See the bug
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
Documentation
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.