Conversation
Reviewer's GuideUpdates Calendar component styling to improve today/selected cell visuals (including dark theme), slightly refactors month/week navigation logic, and cleans up some Razor/table attributes and comments. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new today cell ring styles use several hard-coded pixel/rem units for size and offsets (
width: 2rem,top: -6px,left: -8px), which may not scale well with different font sizes or layouts; consider centering the circle relative to the span (e.g., withtop: 50%,left: 50%,transform: translate(-50%, -50%)) or usingem-based values to make it more robust. - To keep theming concerns localized and easier to maintain, consider nesting the dark theme variable overrides inside the existing
.calendarblock (e.g.,.calendar[data-bs-theme='dark'] { --bb-calendar-selected-bg: ... }or similar) instead of having a separate[data-bs-theme='dark'] .calendarblock at the top.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new today cell ring styles use several hard-coded pixel/rem units for size and offsets (`width: 2rem`, `top: -6px`, `left: -8px`), which may not scale well with different font sizes or layouts; consider centering the circle relative to the span (e.g., with `top: 50%`, `left: 50%`, `transform: translate(-50%, -50%)`) or using `em`-based values to make it more robust.
- To keep theming concerns localized and easier to maintain, consider nesting the dark theme variable overrides inside the existing `.calendar` block (e.g., `.calendar[data-bs-theme='dark'] { --bb-calendar-selected-bg: ... }` or similar) instead of having a separate `[data-bs-theme='dark'] .calendar` block at the top.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7381 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 748 748
Lines 32776 32764 -12
Branches 4546 4546
=========================================
- Hits 32776 32764 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Calendar component's today cell styling to use a circular border indicator instead of just color highlighting. The changes improve visual distinction between today's date and selected dates while adding dark theme support for the selection styling.
Key Changes:
- Introduced a circular border around today's date (visible only when not selected)
- Updated selected cell colors to use white text on blue background for better contrast
- Added dark theme support for selected cells
- Refactored navigation methods to use ternary operators for conciseness
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/wwwroot/scss/variables.scss | Added new SCSS variables for today's border color, updated selected cell colors, and introduced dark theme variant for selected background |
| src/BootstrapBlazor/Components/Calendar/Calendar.razor.scss | Implemented dark theme support, added circular border styling for today's cell using CSS pseudo-element, and registered new CSS variables |
| src/BootstrapBlazor/Components/Calendar/Calendar.razor.cs | Added Chinese documentation comment for the component and refactored month/week navigation methods to use ternary operators |
| src/BootstrapBlazor/Components/Calendar/Calendar.razor | Removed deprecated HTML attributes (cellspacing and cellpadding) from table elements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| width: 2rem; | ||
| height: 2rem; | ||
| top: -6px; | ||
| left: -8px; | ||
| border-radius: 50%; | ||
| border: 1px solid var(--bb-calendar-today-border-color); |
There was a problem hiding this comment.
The border circle uses hardcoded pixel values for positioning (top: -6px, left: -8px) and dimensions (width: 2rem, height: 2rem). This creates a tight coupling between the pseudo-element size and position. Consider using CSS variables or calc() to make the positioning relative to the element's dimensions, or add a comment explaining these magic numbers to improve maintainability.
Link issues
fixes #7356
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Update Calendar component styling to improve the visual treatment of the current day and selected dates, including dark theme support.
New Features:
Enhancements: