-
-
Notifications
You must be signed in to change notification settings - Fork 174
fix(BTable): make background sort icons customizable + docs #2933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
|
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 two demo Vue components showing CSS-variable and header-slot methods to customize BTable sort icons, updates BTable SCSS to use CSS-variable fallbacks and larger 24×24 SVG data-URIs and increases icon size from 1em to 1.5em, and expands table documentation with a "Customizing Sort Icons" section. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)apps/docs/src/docs/**/*.md📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-08-18T18:20:08.240ZApplied to files:
📚 Learning: 2025-09-30T23:57:21.526ZApplied to files:
📚 Learning: 2025-08-19T23:30:07.062ZApplied to files:
📚 Learning: 2025-08-19T23:30:07.062ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the BTable component's sort icon customization capabilities by introducing CSS custom properties and comprehensive documentation. The changes make it easier for developers to customize sort icons to match their design system without modifying component source code.
- Introduces CSS custom properties (
--bvn-sort-icon-none,--bvn-sort-icon-asc,--bvn-sort-icon-desc) for easy sort icon customization - Increases default sort icon size from 1em to 1.5em (SVG dimensions from 16x16 to 24x24) for improved visibility
- Adds comprehensive documentation with two demo approaches: CSS custom properties and scoped slots
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/bootstrap-vue-next/src/components/BTable/_table.scss | Introduces CSS custom properties for sort icons with fallback to default SVGs; increases icon sizes for better UX |
| apps/docs/src/docs/components/table.md | Adds detailed documentation section for customizing sort icons including CSS and slot-based approaches |
| apps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue | Demonstrates customizing sort icons using CSS custom properties with example SVG data URIs |
| apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vue | Demonstrates customizing sort icons using scoped slots with custom SVG content and event handling |
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vue
Outdated
Show resolved
Hide resolved
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/docs/src/docs/components/table.md (1)
811-813: Consider clarifying the thAttr access pattern.The documentation states that
fieldincludesthAttr['aria-sort'], but doesn't mention thatthAttrcan be a function or undefined, requiring type checking. The demo correctly implements the type guard (lines 80-83 in TableSortCustomSlotIcons.vue), but users following just the docs might encounter runtime errors.Consider adding a brief note:
-The scoped slot provides access to the `field` object, which includes `thAttr['aria-sort']` indicating the current sort state (`'none'`, `'ascending'`, or `'descending'`). +The scoped slot provides access to the `field` object, which includes `thAttr['aria-sort']` indicating the current sort state (`'none'`, `'ascending'`, or `'descending'`). Note that `thAttr` may be a function or undefined, so type checking is recommended (see demo for example).apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vue (2)
12-20: Consider extracting inline styles for maintainability.The custom header slots correctly demonstrate the pattern, but the repeated inline styles could be extracted to a style block or CSS class for better maintainability.
Add a style section:
<style scoped> .sortable-header { display: flex; align-items: center; gap: 0.5rem; cursor: pointer; } </style>Then simplify the slots:
<template #head(first_name)="data"> <div class="sortable-header"> {{ data.label }} <span v-if="getSortIcon(data.field)" v-html="getSortIcon(data.field)" /> </div> </template>Also applies to: 23-31, 34-42
92-96: Remove or comment out console.log for production use.The
console.logstatement is useful for demonstration but should be removed or commented out in the final demo to avoid confusion. Alternatively, add a comment indicating it's for demonstration purposes only.function onHeadClicked(key: string, field: TableField<Person>, event: MouseEvent, isfoot: boolean) { // Sorting is handled automatically by BTable // This event is available if you need additional custom logic // console.log('Header clicked:', key, field, isfoot) // Demo only }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue(1 hunks)apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vue(1 hunks)apps/docs/src/docs/components/table.md(1 hunks)packages/bootstrap-vue-next/src/components/BTable/_table.scss(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use conventional commit format with prefixes like
feat:,fix:,docs:, etc. for all commits
Files:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue
apps/docs/src/docs/**/demo/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Demo files must follow structure with template first, then script, then style. Template-only examples must be wrapped in
<!-- #region template -->and<!-- #endregion template -->comments. Use unique IDs for all components to avoid conflicts, keep examples focused on one feature, and use descriptive filenames likeComponentFeature.vue
Files:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue
apps/docs/src/docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Reference demo files using
<<< DEMO ./demo/MyComponent.vue{vue}syntax for full file display or with#region-namemarkers for specific sections
Files:
apps/docs/src/docs/components/table.md
🧠 Learnings (15)
📓 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-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/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue
📚 Learning: 2025-11-30T17:09:16.276Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-30T17:09:16.276Z
Learning: Applies to apps/docs/src/docs/**/demo/*.vue : Demo files must follow structure with template first, then script, then style. Template-only examples must be wrapped in `<!-- #region template -->` and `<!-- #endregion template -->` comments. Use unique IDs for all components to avoid conflicts, keep examples focused on one feature, and use descriptive filenames like `ComponentFeature.vue`
Applied to files:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue
📚 Learning: 2025-11-30T17:09:16.276Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-30T17:09:16.276Z
Learning: Applies to packages/bootstrap-vue-next/src/components/index.ts : When adding new components, add them to the components/index.ts export file
Applied to files:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.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:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vuepackages/bootstrap-vue-next/src/components/BTable/_table.scss
📚 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/demo/TableSortCustomSlotIcons.vuepackages/bootstrap-vue-next/src/components/BTable/_table.scss
📚 Learning: 2025-11-30T17:09:16.276Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-30T17:09:16.276Z
Learning: Applies to apps/docs/src/docs/**/*.md : Reference demo files using `<<< DEMO ./demo/MyComponent.vue{vue}` syntax for full file display or with `#region-name` markers for specific sections
Applied to files:
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vueapps/docs/src/docs/components/demo/TableSortCustomCSSIcons.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:
apps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue
📚 Learning: 2025-11-30T17:09:16.276Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-30T17:09:16.276Z
Learning: Applies to packages/bootstrap-vue-next/src/components/**/*.vue : Edit component files in `packages/bootstrap-vue-next/src/components/` and test using `packages/bootstrap-vue-next/src/App.vue` with hot-reload via `pnpm --filter bootstrap-vue-next run dev`
Applied to files:
apps/docs/src/docs/components/demo/TableSortCustomCSSIcons.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/table.md
📚 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:
apps/docs/src/docs/components/table.md
📚 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:
apps/docs/src/docs/components/table.md
📚 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/table.md
📚 Learning: 2025-11-30T17:09:16.276Z
Learnt from: CR
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-30T17:09:16.276Z
Learning: Applies to packages/bootstrap-vue-next/src/styles/styles.scss : Main library styles are defined in `packages/bootstrap-vue-next/src/styles/styles.scss`, with component-specific styles typically in component `.vue` files, using Bootstrap 5.3.x as the base CSS framework
Applied to files:
packages/bootstrap-vue-next/src/components/BTable/_table.scss
📚 Learning: 2025-04-27T17:30:42.045Z
Learnt from: unstoppablecarl
Repo: bootstrap-vue-next/bootstrap-vue-next PR: 2671
File: packages/bootstrap-vue-next/src/components/BToast/_toast.scss:1-3
Timestamp: 2025-04-27T17:30:42.045Z
Learning: In global SCSS files for bootstrap-vue-next, `:deep()` combinator should not be used as it only works in Vue's scoped style blocks. Standard CSS/SCSS selectors should be used instead.
Applied to files:
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 (8)
packages/bootstrap-vue-next/src/components/BTable/_table.scss (3)
176-177: LGTM: Icon size increase improves visibility.The 50% increase in sort icon size (from 1em to 1.5em) aligns with the PR objective to improve UX. The
!defaultflag appropriately allows downstream customization.
180-182: LGTM: SVG updates are correct.The SVG data URIs correctly scale from 16x16 to 24x24. The viewBox remains "0 0 16 16" (defining the coordinate system), while width/height attributes are updated to 24x24 (defining display size). This ensures proper scaling without distortion.
214-217: LGTM: CSS variable implementation enables customization.The CSS custom properties (
--bvn-sort-icon-none,--bvn-sort-icon-asc,--bvn-sort-icon-desc) with SCSS fallbacks correctly enable icon customization while maintaining backwards compatibility. Thebv-escape-svg()function is evaluated at compile-time, producing valid CSSurl()values as fallbacks.Also applies to: 221-221, 225-228
apps/docs/src/docs/components/demo/TableSortCustomCSSIcons.vue (3)
1-11: LGTM: Clean demo template.The template effectively demonstrates CSS-based icon customization with a focused example. The
custom-sort-iconsclass provides good scoping for the CSS variables.
13-32: LGTM: Appropriate demo data.The script setup provides a simple, clear example with appropriate sample data. The three sortable columns effectively demonstrate the customization feature.
34-46: LGTM: Effective CSS variable customization demo.The scoped styles correctly demonstrate overriding sort icons via CSS custom properties. The CSS variable names match the SCSS implementation, and the SVG data URIs are properly URL-encoded.
apps/docs/src/docs/components/demo/TableSortCustomSlotIcons.vue (2)
15-18: v-html usage is safe in this demo context.The
v-htmldirective safely renders hardcoded SVG strings. However, if developers adapt this pattern for production use with dynamic icon sources, they should ensure proper sanitization or use component-based icons instead.Also applies to: 26-29, 37-40
78-90: LGTM: Proper type guarding for thAttr access.The
getSortIconfunction correctly implements type guards to safely accessfield.thAttr['aria-sort']. The checks handle both the object-vs-function nature ofthAttrand null cases appropriately.
- 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]>
Describe the PR
closes #2932
Small replication
see examples in 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
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.