fix(scss) moved all scss styles out of components #2671
Conversation
|
|
WalkthroughComponent-scoped style blocks were removed from several Vue component files and their corresponding styles were migrated into dedicated SCSS partials for Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant StylesEntry as styles.scss
participant AlertSCSS as _alert.scss
participant PopoverSCSS as _popover.scss
participant ToastSCSS as _toast.scss
App->>StylesEntry: Import global styles
StylesEntry->>AlertSCSS: Import BAlert styles
StylesEntry->>PopoverSCSS: Import BPopover styles
StylesEntry->>ToastSCSS: Import BToast styles
StylesEntry->>App: Styles applied globally
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
related to #1885 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/bootstrap-vue-next/src/components/BOffcanvas/_offcanvas.scss (1)
1-3: Avoid polluting the global namespace with.no-transition.
The class name.no-transitionis very generic and may unintentionally affect other components or utilities. Consider scoping it under the offcanvas context or renaming it to a BOffcanvas-specific selector, for example:.offcanvas.no-transition { ... }or
.b-offcanvas-no-transition { ... }packages/bootstrap-vue-next/src/components/BToast/_toast.scss (2)
12-18: Remove or clarify trial-and-error comments.
The multiline comment describing unknown animation behaviors adds noise. If you need to revisit or refine this logic, replace it with a concise TODO or remove it before merging.
19-30: Scope list animation styles to avoid global side effects.
The.b-list-*classes are applied globally and could interfere with other transition groups across the app. Consider namespacing them under a toast-specific wrapper (e.g..b-toast-list-enter-active), or nest them inside a.toastor.b-toast-orchestratorcontainer.packages/bootstrap-vue-next/src/components/BAlert/_alert.scss (1)
1-7: Scope.btn-close-customto the.alertcomponent to prevent global style leakage.
By defining these styles globally, any element with.btn-close-customwill receive alert-specific positioning and margin, which may lead to unintended side effects. Consider nesting under the alert selector:--- a/packages/bootstrap-vue-next/src/components/BAlert/_alert.scss +++ b/packages/bootstrap-vue-next/src/components/BAlert/_alert.scss @@ -1,7 +1,11 @@ -.btn-close-custom { +.alert { + .btn-close-custom { position: absolute; top: 0; right: 0; z-index: 2; - margin: var(--bs-alert-padding-y) var(--bs-alert-padding-x); -} + margin: var(--bs-alert-padding-y) var(--bs-alert-padding-x); + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/bootstrap-vue-next/src/components/BAlert/BAlert.vue(0 hunks)packages/bootstrap-vue-next/src/components/BAlert/_alert.scss(1 hunks)packages/bootstrap-vue-next/src/components/BDropdown/BDropdown.vue(0 hunks)packages/bootstrap-vue-next/src/components/BDropdown/_dropdown.scss(1 hunks)packages/bootstrap-vue-next/src/components/BModal/BModal.vue(0 hunks)packages/bootstrap-vue-next/src/components/BModal/_modal.scss(1 hunks)packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue(0 hunks)packages/bootstrap-vue-next/src/components/BOffcanvas/_offcanvas.scss(1 hunks)packages/bootstrap-vue-next/src/components/BPopover/BPopover.vue(0 hunks)packages/bootstrap-vue-next/src/components/BPopover/_popover.scss(1 hunks)packages/bootstrap-vue-next/src/components/BToast/BToast.vue(0 hunks)packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue(0 hunks)packages/bootstrap-vue-next/src/components/BToast/_toast.scss(1 hunks)packages/bootstrap-vue-next/src/styles/styles.scss(1 hunks)
💤 Files with no reviewable changes (7)
- packages/bootstrap-vue-next/src/components/BDropdown/BDropdown.vue
- packages/bootstrap-vue-next/src/components/BModal/BModal.vue
- packages/bootstrap-vue-next/src/components/BToast/BToast.vue
- packages/bootstrap-vue-next/src/components/BAlert/BAlert.vue
- packages/bootstrap-vue-next/src/components/BOffcanvas/BOffcanvas.vue
- packages/bootstrap-vue-next/src/components/BToast/BToastOrchestrator.vue
- packages/bootstrap-vue-next/src/components/BPopover/BPopover.vue
🔇 Additional comments (4)
packages/bootstrap-vue-next/src/components/BDropdown/_dropdown.scss (1)
1-5: Looks good.
The nested SCSS correctly centralizes the.dropdown-menu.fade { display: block; }rule without any obvious side effects.packages/bootstrap-vue-next/src/styles/styles.scss (1)
13-18: Component style imports look correct.
The new imports for BAlert, BDropdown, BModal, BOffcanvas, BPopover, and BToast integrate all relocated SCSS partials into the global stylesheet in a consistent order.packages/bootstrap-vue-next/src/components/BModal/_modal.scss (1)
5-7: Verify z-index against the backdrop and other modals.
You’ve set.modal-dialog { z-index: 1051; }, but Bootstrap’s default backdrop is1050and the modal container is typically1055. Please confirm this ordering achieves the intended stacking without visual glitches.packages/bootstrap-vue-next/src/components/BPopover/_popover.scss (1)
1-5:❌ Incorrect review comment
Fix the broken selector by adding the missing descendant combinator.
The selector on lines 1–2 is split without a space, causing it to compile into.bs-popover-auto[data-popper-placement^='bottom'].popover-arrowand not match the intended element. Update to:--- a/packages/bootstrap-vue-next/src/components/BPopover/_popover.scss +++ b/packages/bootstrap-vue-next/src/components/BPopover/_popover.scss @@ -1,5 +1,5 @@ -.bs-popover-auto[data-popper-placement^='bottom'] -.popover-arrow:has(+ div > .popover-header)::after, +.bs-popover-auto[data-popper-placement^='bottom'] .popover-arrow:has(+ div > .popover-header)::after, .bs-popover-bottom .popover-arrow:has(+ div > .popover-header)::after { --bs-popover-bg: var(--bs-popover-header-bg); }Also verify that the
:has()pseudo-class is supported in your target browsers, or provide a fallback for environments where it isn’t available.
🏁 Script executed:
#!/bin/bash set -e # Locate the SCSS file file=$(fd --hidden --exact-depth 6 '_popover.scss' | head -n1) echo "File path: $file" echo "----- First 5 lines of the file -----" sed -n '1,5p' "$file"Length of output: 690
Descendant combinator is already implied
SCSS treats newlines as whitespace in selectors, so.bs-popover-auto[data-popper-placement^='bottom'] .popover-arrow:has(+ div > .popover-header)::after { … }already compiles to
.bs-popover-auto[data-popper-placement^='bottom'] .popover-arrow:has(+ div > .popover-header)::afterNo changes required to the selector.
Verify
:has()support
Ensure your target browsers support the CSS:has()pseudo-class or provide a fallback for environments where it isn’t available.Likely an incorrect or invalid review comment.
commit: |
packages/bootstrap-vue-next/src/components/BDropdown/_dropdown.scss
Outdated
Show resolved
Hide resolved
|
also in #2556 I tested a way to get the css var from modal. Now that we are doing this we should add the --bs-modal-zindex and --bs-toast-max-width var from variables.scss to the :root I don't think we need to support custom bootstrap #{$prefix} values? |
packages/bootstrap-vue-next/src/components/BOffcanvas/_offcanvas.scss
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
571c67d to
8e929bc
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
* upstream/main: chore: remove blur from boverlay in cases where its not allowed by br… (bootstrap-vue-next#2712) feat(BTableLite): add `table-colgroup` slot (bootstrap-vue-next#2680) perf: use getter functions over computed in some cases feat(BModal): use css var for zindex, add helper vars and ontop class (bootstrap-vue-next#2556) feat(BFormTags): added feedbackAriaLive prop (bootstrap-vue-next#2696) fix(scss) moved all scss styles out of components (bootstrap-vue-next#2671) fix(BModal): ensure clicking inside and releasing outside does not close modal (bootstrap-vue-next#2703) (bootstrap-vue-next#2704) fix(BToast): race condition if using setInterval to update countdown doc data feat(BPopover): add titleClass and bodyClass, remove unneeded customClass prop since class is inherited to the same place feat(BToast): add noProgress prop, make progress show as default if modelValue is number. fix(useToastController): if using the deprecated show method the countdown didn't start.
Describe the PR
Moved all styles from components into separate scss files. The following components were modified:
BAlert
BDropdown
BModal
BOffcanvas
BPopover
BToast
BToastOrchestrator
I did my best to follow the existing style and file structure. Please advise if there are any mistakes.
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 deniedI am not sure if this would be considered a breaking change for the scoped styles.
Summary by CodeRabbit