Conversation
Reviewer's GuideThis PR refactors the CSS for the SelectRegion and SelectCity components by replacing hardcoded style values with CSS custom properties and adds dark‐theme support via [data-bs-theme="dark"] overrides. Class diagram for CSS custom properties in SelectRegion and SelectCity componentsclassDiagram
class SelectRegionCSS {
+--bb-region-header-padding
+--bb-region-header-hover-bg-color
+--bb-region-header-active-bg-color
+--bb-region-body-color
+--bb-region-body-hover-bg-color
+--bb-region-body-active-bg-color
+--bb-region-body-hover-color
+--bb-region-body-active-color
+--bb-region-body-width
+--bb-region-body-padding
+--bb-region-body-item-padding
+--bb-region-body-gap
+[data-bs-theme="dark"] overrides
}
class SelectCityCSS {
+--bb-region-body-color
+--bb-region-body-hover-bg-color
+--bb-region-body-active-bg-color
+--bb-region-body-hover-color
+--bb-region-body-active-color
+--bb-region-body-width
+--bb-region-body-height
+--bb-region-body-padding
+--bb-region-body-item-padding
+--bb-region-body-gap
+[data-bs-theme="dark"] overrides
}
SelectRegionCSS <|-- SelectCityCSS
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
Adds dark theme support to Region components by introducing CSS custom properties and theme overrides.
- Replaces hard-coded colors/sizes with CSS variables for easier theming.
- Adds [data-bs-theme="dark"] overrides for both SelectRegion and SelectCity dropdowns.
- Bumps package version to 9.0.2.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/components/BootstrapBlazor.Region/Components/SelectRegion.razor.css | Introduces CSS variables and dark theme overrides; refactors header/body styles to use variables. |
| src/components/BootstrapBlazor.Region/Components/SelectCity.razor.css | Introduces CSS variables and dark theme overrides; changes dropdown dimensions from max-* to fixed width/height. |
| src/components/BootstrapBlazor.Region/BootstrapBlazor.Region.csproj | Version bump from 9.0.1 to 9.0.2. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| width: var(--bb-region-body-width); | ||
| height: var(--bb-region-body-height); |
There was a problem hiding this comment.
This change replaces the previous max-width/max-height with fixed width/height, which alters the component's behavior (potentially introducing excess whitespace for small content and constraining growth for larger content). To preserve prior behavior and avoid a breaking visual change, use max-width and max-height with the new variables.
| width: var(--bb-region-body-width); | |
| height: var(--bb-region-body-height); | |
| max-width: var(--bb-region-body-width); | |
| max-height: var(--bb-region-body-height); |
| background-color: var(--bb-region-header-active-bg-color); | ||
| color: var(--bb-region-body-active-color); | ||
| } | ||
|
|
||
| .bb-region-header ul li:hover { | ||
| background-color: #e3e3e3; | ||
| color: #000; | ||
| background-color: var(--bb-region-header-hover-bg-color); | ||
| color: var(--bb-region-body-hover-color); |
There was a problem hiding this comment.
[nitpick] Header state text colors are sourced from body variables, which couples header styling to body settings. Consider introducing header-specific text color variables (e.g., --bb-region-header-hover-color and --bb-region-header-active-color) and referencing them here to keep header styling self-contained and easier to theme independently.
| .bb-region-body ul li:hover { | ||
| background-color: #e9ecef; | ||
| color: #000; | ||
| background-color: var(--bb-region-body-hover-bg-color); | ||
| color: var(--bb-region-body-hover-color); | ||
| } |
There was a problem hiding this comment.
[nitpick] Interactive items have hover and active states but no visible keyboard focus state. Add :focus-visible styles aligned with hover for keyboard accessibility, e.g., .bb-region-body ul li:focus-visible { background-color: var(--bb-region-body-hover-bg-color); color: var(--bb-region-body-hover-color); outline: 0; } and similarly for .bb-region-header ul li.
| background-color: var(--bb-region-body-hover-bg-color); | ||
| color: var(--bb-region-body-hover-color); | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] Add a :focus-visible state for .dropdown-menu li to match hover styling so keyboard users can see focus, e.g., .dropdown-menu li:focus-visible { background-color: var(--bb-region-body-hover-bg-color); color: var(--bb-region-body-hover-color); outline: 0; }.
| .dropdown-menu li:focus-visible { | |
| background-color: var(--bb-region-body-hover-bg-color); | |
| color: var(--bb-region-body-hover-color); | |
| outline: 0; | |
| } |
| --bb-region-header-padding: 3px 12px; | ||
| --bb-region-header-hover-bg-color: #e9ecef; | ||
| --bb-region-header-active-bg-color: #dee2e6; | ||
| --bb-region-body-color: #495057; | ||
| --bb-region-body-hover-bg-color: #e9ecef; | ||
| --bb-region-body-active-bg-color: #dee2e6; | ||
| --bb-region-body-hover-color: #000; | ||
| --bb-region-body-active-color: #000; | ||
| --bb-region-body-width: 400px; | ||
| --bb-region-body-padding: .5rem; | ||
| --bb-region-body-item-padding: 3px 12px; | ||
| --bb-region-body-gap: 5px; |
There was a problem hiding this comment.
[nitpick] New CSS custom properties form part of the component's theming surface. Please document each variable (purpose, default value, and examples, including dark theme overrides), so consumers know how to customize Region styling.
Link issues
fixes #605
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable dark theme styling for region selection components by introducing CSS custom properties and providing dynamic overrides for light and dark modes.
New Features:
Enhancements: