Dark mode for Admin Panel#787
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds a dark mode feature to the admin panel, allowing users to toggle between light and dark themes with persistence across sessions. The implementation uses a cookie-based state management system to ensure the correct theme is applied during server-side rendering, preventing visual flashes. It leverages CSS variables for theme-specific styling, providing a robust and maintainable way to override Bootstrap 4 components without requiring a full framework upgrade. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a theme selection feature (light and dark modes) for the admin panel, adding a ThemeSelector component, a ThemeController to persist the selection in a cookie, and a theme.css stylesheet with CSS variables. Feedback on the changes highlights three critical issues: first, ThemeSelector incorrectly uses constructor injection which is unsupported in Blazor and will cause a runtime exception; second, the redirect URI in ThemeSelector lacks a leading slash, causing 404 errors on nested routes; and third, ThemeController needs to validate the redirectUri parameter to prevent unhandled exceptions from invalid or external URLs.
Cookie-persisted dark/light theme toggle in the panel header, mirroring the CultureSelector pattern. ThemeController writes an OpenMU.Theme cookie; App.razor reads it on SSR to set <html data-theme="…">; theme.css and CSS-isolation files drive the colors via CSS variables. ThemeSelector hydrates from the live DOM via a small JS module after the interactive Blazor circuit takes over (cascading HttpContext is null in interactive renders). theme.css is shipped as a plain static asset rather than going through the SCSS pipeline because the Docker build skips Sass with ci=true.
drop the "Pre-existing regression" section (Eduardo's MUnique#788 now owns that fix) Review feedback (3 items): - ThemeSelector: drop the constructor, inject NavigationManager as a property to match the [Inject] pattern used by the rest of the project (MapEditor, ConfigurationSearch, NavMenu, ...). - ThemeSelector: prepend a leading slash to the redirect target ("/Theme/Set?...") so it always resolves against the application root, independently of the current route depth. - ThemeController: accept a nullable redirectUri and fall back to LocalRedirect("/") when it is missing or not a local URL, so a malformed request can no longer crash LocalRedirect into a 500. Dark-mode coverage gaps: - theme.css: set `color-scheme: dark` under [data-theme="dark"] so native form controls (unchecked checkboxes/radios, scrollbars, date pickers) render in their dark variant instead of bright white. - Typeahead.razor.css: replace hardcoded #fff/#ced4da/#6c757d/... with the --omu-* variables. Covers every LookupField / MultiLookupField / Typeahead in the panel (auto-form lookups on ItemDefinition, Skill, MonsterDefinition, ..., plus the Connect Server config, item-slot pickers, modal selectors). - theme.css: shift --omu-sidebar-gradient stops from % to vh and add an override for `.sidebar > nav` in dark mode (Navigation.scss hardcodes the light gradient there, would otherwise leak into dark mode). - theme.css: add an override for `.sidebar > nav` in dark mode (Navigation.scss hardcodes the light gradient there, would otherwise leak into dark mode); add an override for `.creation-panel-body .form-actions` introduced by MUnique#788 so the new sticky footer doesn't stay bright white in dark mode.
|
Thanks for the careful review. Rebased onto current master (which 1. Constructor injection on ThemeSelector 2. Relative 3. Heads-up on follow-ups Comments #2 and #3 also apply verbatim to the existing Other changes in this push (not from review) While testing the final build I noticed three things worth fixing:
|
QuickGrid header/sort/paginator, alert variants, sticky bars
QuickGrid:
- button.col-title + .col-title-text: shared.css globally sets
`button { color: #212529 }` which leaks to the bare <button> used by
the QuickGrid sort header. Narrow override so `.btn-*` variants
(which have their own explicit color) are not affected.
- .sort-indicator and .go-first/previous/next/last: QuickGrid renders
these arrows as SVG data-URIs in `background-image` with no `fill`
attribute, defaulting to black. Can't override SVG fill from outside;
use `filter: invert(1)` so black -> white in dark mode.
Sticky bars added by MUnique#788:
- .add-new-bar (EditConfigGrid.razor.css) hardcodes background-color
#fff and a #dee2e6 top border — same pattern we already overrode for
`.creation-panel-body .form-actions`. Added the analogous override.
Modals:
- .blazored-modal (Blazored.Modal NuGet package, `_content/Blazored.Modal/
blazored.modal.bundle.scp.css`) hardcodes `background-color: #fff` and
white border. Override the panel surface; backdrop is already
`rgba(0, 0, 0, 0.5)` so it works on both themes.
- .multi-lookup-field__* (OpenMU's own multi-select) had hardcoded
Bootstrap defaults across container, tag, dropdown, hover and selected
states. Mapped to --omu-* tokens — fixes the Plugins config modal
(gear icon on a plugin) and the Excellent / Wing options pickers in
ItemEdit.
Bootstrap alert variants (used on Updates.razor and Install.razor):
- .alert-primary, .alert-success, .alert-info, .alert-warning,
.alert-danger, .alert-light: light backgrounds + dark text are
illegible on dark mode. Subtle tinted backgrounds (15% accent over
the dark surface) with bright accent text and 40% accent border,
per Bootstrap dark-mode conventions. .alert-secondary stays as it
was overridden in the earlier commit.
- `.alert hr`: Bootstrap sets per-variant `<hr>` border colors as bright
tints of the alert hue (e.g. .alert-primary hr -> rgb(158, 204, 255));
too loud over our dark alert backgrounds. One neutral 15%-white line
for all variants.
|
Pushed one more commit with dark-mode polish I caught while visually Coverage gaps fixed:
Verified each rule lands in the served Note: this also indirectly covers a Verification reference (what was checked, for reviewers)Built locally via the project's docker compose, hit the running admin Monsters / Skills / Items / Character classes — Name header, sort |
|
Ready to merge. |
1 participant
Dark mode for Admin Panel
Summary
Adds a dark-mode toggle to the admin panel with persistence across sessions and page reloads. Implementation mirrors the existing
CultureSelectorpattern (cookie-based, no SignalR state), so the architectural footprint is minimal and the mental model is consistent with what already exists in the codebase.The toggle lives in the panel header next to the "About" link. State is stored in an
OpenMU.Themecookie. A[data-theme]attribute on<html>drives all styling via CSS custom properties.What's in
New files
src/Web/Shared/Services/ThemeController.cs— MVC controller endpointTheme/Set?theme=…&redirectUri=…(1:1 withCultureController)src/Web/Shared/Components/ThemeSelector.razor+.razor.cs+.razor.css— toggle button with moon/sun icons from open-iconicsrc/Web/Shared/wwwroot/themeSelector.js— tinyexport function current()readingdocument.documentElement.dataset.themefor hydration after the Blazor interactive circuit takes oversrc/Web/Shared/wwwroot/css/theme.css— theme tokens (:root/[data-theme="dark"]) and Bootstrap 4 surface overrides (.bg-light, tables, forms, dropdowns, modals, sidebar dropdown submenu)Modified files
src/Web/AdminPanel/Components/App.razor— sets<html data-theme="…">from the cookie via[CascadingParameter] HttpContextsrc/Web/AdminPanel/Components/Layout/MainLayout.razor+.razor.css— adds the<ThemeSelector>to the header, recolors sidebar gradient/breadcrumb/#blazor-error-uivia CSS variablessrc/Web/AdminPanel/Components/Layout/CreationPanel.razor.css,ConfigurationSearch.razor.css— surface colors switched to CSS variablessrc/Web/Shared/Components/Form/AutoForm.razor.css— Save/Refresh action bar background switched to CSS variables (fixes light panel under buttons on every config edit page)src/Web/Shared/Exports.cs—theme.cssadded toStylesheetssrc/Web/Shared/Properties/Resources.resx+Resources.Designer.cs— three new strings:ToggleTheme,SwitchToDarkMode,SwitchToLightModesrc/Web/AdminPanel/_Imports.razor—@using Microsoft.AspNetCore.Httpso the cascadingHttpContextresolvesDesign decisions
Cookie + reload (not localStorage + JS swap). Identical pattern to
CultureSelector. Keeps SSR honest — the first paint matches the active theme, no flash-of-light-content. Cost: one full reload per toggle, accepted in admin tooling (low traffic, infrequent action).CSS variables in a separate
theme.css, not in the SCSS pipeline. The Dockerfile runsdotnet build -p:ci=true, which skips theCompileSassMSBuild target. SCSS edits never reach the publishedshared.cssin containerized builds.theme.cssis a plain static asset loaded viaExports.Stylesheetsand works on every host.Bootstrap 4 surface overrides (not an upgrade to BS5). Bootstrap 5 ships native dark-mode via
data-bs-theme, but the migration is a separate, much larger effort. Overriding the specific BS4 classes the panel uses (.bg-light,.table-striped,.dropdown-menu,.form-control,.modal-content, …) is the smallest viable change.JS hydration via
wwwroot/themeSelector.jsas an ES module, not colocated.razor.js. Colocated*.razor.jsfiles are 404 in this project's build pipeline (verified —ReconnectModal.razor.jsis also 404; see related issues).wwwroot/is the reliable static-asset path, same asshared.cssand our newtheme.css. The hydration readsdata-themefrom<html>after the interactive circuit connects, because[CascadingParameter] HttpContextis null in interactive renders — without this, the toggle would get stuck in dark mode (the icon would flip back to "moon" while the cookie stayeddark).Icon convention: action-affordance. Moon icon shown in light mode (click → go dark), sun icon shown in dark mode (click → go light). Same convention as iOS, GitHub, Stripe Dashboard, Material Design. Tooltip ("Switch to dark mode" / "Switch to light mode") reinforces the affordance.
Known limitations / out-of-scope
PersistentComponentState; not worth the boilerplate for an admin tool..resxfiles yet, so consistent with existing convention.MUnique.OpenMU.Web.AdminPanel.csprojSDK reference chain — not touched. Pre-existing static-web-asset path-doubling issue (_content/AdminPanel/_content/<pkg>/…404s) and theReconnectModal.razor.js404 are pre-existing bugs unrelated to dark mode, surfaced in the same DevTools sweep. Tracked separately (see related issues).Test plan
localhost:8082, confirm panel loads in light mode by defaultAutoFormpanel and the sticky Save/Refresh bar are dark<a class="btn btn-info">) has white text on teal background, readable<button class="btn-info">) is consistentOpenMU.Theme = darkis set in DevTools → Application → Cookies after toggling<html data-theme="dark">is set in DevTools → Elements after togglingPreview:
