Style app shell and route partials in admin dashboard#10
Open
fallofpheonix wants to merge 1 commit into
Open
Conversation
base.css targeted the original map-centric markup; the index.html shell (.app/.topbar/.layout/.content) and route partial classes had no styles and rendered as unstyled text. Adds a shell layout patch reusing the existing design tokens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer's GuideAdds a new shell-layout section to the admin dashboard base.css to style the index app shell and dashboard route partials using existing design tokens, turning the previously unstyled dashboard into a structured, card-based command-center layout. Flow diagram for new admin dashboard shell layout structureflowchart TD
App[app]
Topbar[topbar]
Layout[layout]
Sidebar[sidebar]
Content[content]
Grid[dashboard-grid]
SummaryCards[summary-cards]
Card[card / metric]
PlaceholderMap[placeholder-map]
QuickActions[quick-actions]
App --> Topbar
App --> Layout
Layout --> Sidebar
Layout --> Content
Content --> Grid
Grid --> SummaryCards
Grid --> PlaceholderMap
Grid --> QuickActions
SummaryCards --> Card
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis change appends a new CSS section to base.css defining app shell layout, topbar, sidebar, and dashboard content styles (cards, metrics, feed items, quick actions) for index.html route partials, without altering any existing declarations. ChangesShell Layout CSS
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
.layout .sidebaruses thegapproperty without a flex or grid display; if spacing between children is intended, consider addingdisplay: flexordisplay: gridsogaptakes effect.- The fixed
height: 180pxon.placeholder-mapmay feel cramped or overly tall on different viewports; consider making this responsive (e.g., usingmin-heightwith flex growth or relative units) to better adapt to varying screen sizes. - Interactive elements like
.quick-actions buttononly have hover styles; adding visible:focusstates will improve keyboard accessibility and make the dashboard easier to navigate without a mouse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- `.layout .sidebar` uses the `gap` property without a flex or grid display; if spacing between children is intended, consider adding `display: flex` or `display: grid` so `gap` takes effect.
- The fixed `height: 180px` on `.placeholder-map` may feel cramped or overly tall on different viewports; consider making this responsive (e.g., using `min-height` with flex growth or relative units) to better adapt to varying screen sizes.
- Interactive elements like `.quick-actions button` only have hover styles; adding visible `:focus` states will improve keyboard accessibility and make the dashboard easier to navigate without a mouse.
## Individual Comments
### Comment 1
<location path="dashboard-admin/css/base.css" line_range="738-741" />
<code_context>
+ min-height: 0;
+}
+
+.layout .sidebar {
+ width: var(--sidebar-width);
+ flex-shrink: 0;
+ gap: 6px;
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `gap` property on `.layout .sidebar` will have no effect unless the sidebar is a flex or grid container.
Confirm whether `.layout .sidebar` is defined as a flex or grid container elsewhere. If not, `gap` will be ignored. If the goal is vertical spacing between sidebar items, consider making the sidebar a flex/grid container or applying margins to the child elements instead.
</issue_to_address>
### Comment 2
<location path="dashboard-admin/css/base.css" line_range="867-875" />
<code_context>
+ gap: 12px;
+}
+
+.quick-actions button {
+ padding: 12px 22px;
+ border-radius: 12px;
+ border: 1px solid var(--glass-border);
+ background: linear-gradient(110deg, rgba(0, 210, 255, 0.18), rgba(157, 80, 187, 0.18));
+ color: var(--text-primary);
+ font-size: 0.9rem;
+ cursor: pointer;
+ transition: filter 0.2s;
+}
+
</code_context>
<issue_to_address>
**suggestion:** Buttons have a hover state but no focus-visible treatment, which can hurt keyboard accessibility.
Only `:hover` is styled, so keyboard users may rely on inconsistent or overridden default focus styles. Please add a clear `:focus-visible` treatment (e.g., outline or box-shadow) aligned with the design to make these actions accessible to keyboard navigation.
Suggested implementation:
```
.quick-actions {
grid-column: 1 / -1;
display: flex;
gap: 12px;
}
.quick-actions button {
padding: 12px 22px;
border-radius: 12px;
border: 1px solid var(--glass-border);
background: linear-gradient(110deg, rgba(0, 210, 255, 0.18), rgba(157, 80, 187, 0.18));
color: var(--text-primary);
font-size: 0.9rem;
cursor: pointer;
transition: filter 0.2s, box-shadow 0.2s;
}
/* Mouse hover state */
.quick-actions button:hover {
filter: brightness(1.08);
}
/* Keyboard focus-visible state for accessibility */
.quick-actions button:focus-visible {
outline: none;
box-shadow: 0 0 0 2px rgba(0, 210, 255, 0.75);
}
.app {
```
1. If there is already a `.quick-actions button` rule or `.quick-actions button:hover` rule elsewhere in `base.css`, merge these declarations instead of duplicating them to avoid conflicting styles.
2. You may want to replace the hard-coded `rgba(0, 210, 255, 0.75)` with a design token (e.g., `var(--accent)` or a dedicated focus color variable) if such a variable exists in your design system to keep the focus ring consistent with the rest of the app.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+738
to
+741
| .layout .sidebar { | ||
| width: var(--sidebar-width); | ||
| flex-shrink: 0; | ||
| gap: 6px; |
There was a problem hiding this comment.
issue (bug_risk): The gap property on .layout .sidebar will have no effect unless the sidebar is a flex or grid container.
Confirm whether .layout .sidebar is defined as a flex or grid container elsewhere. If not, gap will be ignored. If the goal is vertical spacing between sidebar items, consider making the sidebar a flex/grid container or applying margins to the child elements instead.
Comment on lines
+867
to
+875
| .quick-actions button { | ||
| padding: 12px 22px; | ||
| border-radius: 12px; | ||
| border: 1px solid var(--glass-border); | ||
| background: linear-gradient(110deg, rgba(0, 210, 255, 0.18), rgba(157, 80, 187, 0.18)); | ||
| color: var(--text-primary); | ||
| font-size: 0.9rem; | ||
| cursor: pointer; | ||
| transition: filter 0.2s; |
There was a problem hiding this comment.
suggestion: Buttons have a hover state but no focus-visible treatment, which can hurt keyboard accessibility.
Only :hover is styled, so keyboard users may rely on inconsistent or overridden default focus styles. Please add a clear :focus-visible treatment (e.g., outline or box-shadow) aligned with the design to make these actions accessible to keyboard navigation.
Suggested implementation:
.quick-actions {
grid-column: 1 / -1;
display: flex;
gap: 12px;
}
.quick-actions button {
padding: 12px 22px;
border-radius: 12px;
border: 1px solid var(--glass-border);
background: linear-gradient(110deg, rgba(0, 210, 255, 0.18), rgba(157, 80, 187, 0.18));
color: var(--text-primary);
font-size: 0.9rem;
cursor: pointer;
transition: filter 0.2s, box-shadow 0.2s;
}
/* Mouse hover state */
.quick-actions button:hover {
filter: brightness(1.08);
}
/* Keyboard focus-visible state for accessibility */
.quick-actions button:focus-visible {
outline: none;
box-shadow: 0 0 0 2px rgba(0, 210, 255, 0.75);
}
.app {
- If there is already a
.quick-actions buttonrule or.quick-actions button:hoverrule elsewhere inbase.css, merge these declarations instead of duplicating them to avoid conflicting styles. - You may want to replace the hard-coded
rgba(0, 210, 255, 0.75)with a design token (e.g.,var(--accent)or a dedicated focus color variable) if such a variable exists in your design system to keep the focus ring consistent with the rest of the app.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dashboard-admin/css/base.css (1)
744-756: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd keyboard focus states for sidebar links and quick-action buttons.
Only
:hoveris styled; no explicit:focus/:focus-visiblerule is provided for.layout .sidebar aor.quick-actions button, which can hurt keyboard-only navigation if a global outline reset exists elsewhere in the stylesheet.♿ Suggested addition
.layout .sidebar a:hover { background: var(--card-bg); color: var(--text-primary); } + +.layout .sidebar a:focus-visible { + background: var(--card-bg); + color: var(--text-primary); + outline: 2px solid var(--accent-blue); +}.quick-actions button:hover { filter: brightness(1.25); } + +.quick-actions button:focus-visible { + outline: 2px solid var(--accent-blue); +}Also applies to: 867-876
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard-admin/css/base.css` around lines 744 - 756, Add explicit keyboard focus styling for the sidebar and quick actions, since only hover states are defined. Update the existing `.layout .sidebar a` and `.quick-actions button` rules to include `:focus`/`:focus-visible` alongside `:hover`, using the same visual treatment or a clearly visible focus ring so keyboard users can see the active element.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@dashboard-admin/css/base.css`:
- Around line 744-756: Add explicit keyboard focus styling for the sidebar and
quick actions, since only hover states are defined. Update the existing `.layout
.sidebar a` and `.quick-actions button` rules to include
`:focus`/`:focus-visible` alongside `:hover`, using the same visual treatment or
a clearly visible focus ring so keyboard users can see the active element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06987cb5-0de3-46cd-b715-f003f5f14afd
📒 Files selected for processing (1)
dashboard-admin/css/base.css
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3cb7256d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+825
to
+829
| .content .metric { | ||
| font-family: var(--font-heading); | ||
| font-size: 2.2rem; | ||
| font-weight: 700; | ||
| } |
There was a problem hiding this comment.
When the Diagnostics route is loaded into <main class="content">, system-health.html also contains several <div class="metric"> rows for the service details, so this broad selector turns those label/value rows into 2.2rem bold headline text. This selector appears intended for the dashboard summary numbers; scoping it to .risk-card .metric or .summary-cards .metric would avoid breaking the health page layout.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
base.css targeted the original map-centric markup; the index.html shell (.app/.topbar/.layout/.content) and the route partial classes (.summary-cards, .risk-card, .placeholder-map, .quick-actions, …) had no styles, so the dashboard rendered as unstyled text on a dark background.
This adds a commented shell-layout patch section to base.css reusing the existing design tokens (glass cards, accent gradients, risk colors). Verified locally: dashboard route now renders as a dark command-center layout with styled sidebar, topbar, metric cards, heatmap placeholder, and action buttons.
The map/routes/analytics partials still need equivalent treatment.
🤖 Generated with Claude Code
Summary by Sourcery
Style the admin dashboard app shell and route partials to match the existing dark command-center design.
New Features:
Summary by CodeRabbit