Merged
Conversation
- Wire up the existing default-dark.css that was never loaded. Add a three-way theme switch (System/Light/Dark) that persists the user's preference in localStorage. - The toggle appears in as a dropdown on the plugin directory and details pages. - System mode follows the OS preference automatically. - Also fix the plugin detail page header using hardcoded light colors that didn't adapt to dark mode.
|
Note
|
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'feat: Theme toggle' clearly and concisely describes the main change: adding a theme toggle feature to the application. |
| Description check | ✅ Passed | The description is directly related to the changeset, explaining the theme toggle implementation with details about localStorage persistence, the three-way toggle, dropdown placement, and the hardcoded color fix. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
PluginBuilder/wwwroot/main/layout.css (1)
233-240: Minor: Stylelint formatting preference.Stylelint flags a missing empty line before line 235. This is a stylistic preference that can be addressed if the project enforces this rule.
✨ Fix Stylelint warning
.btcpay-theme-switch-themes button { --icon-size: 1.25rem !important; + background: none; padding: 0; border: 0; color: var(--btcpay-body-text-muted); cursor: pointer; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/wwwroot/main/layout.css` around lines 233 - 240, Add a blank line before the .btcpay-theme-switch-themes button rule to satisfy Stylelint formatting preference; locate the CSS selector ".btcpay-theme-switch-themes button" in layout.css and insert a single empty line immediately above that rule so the file conforms to the project's Stylelint spacing rule.PluginBuilder/Components/MainNav/Default.cshtml (1)
172-181: Consider adding ARIA attributes for improved accessibility.The theme switch buttons lack
aria-labelattributes andaria-pressedstate indicators. Screen reader users would benefit from knowing which theme is currently active.♿ Suggested accessibility improvement
<li class="border-top py-1 px-3"> <div class="btcpay-theme-switch w-100 py-2"> - <span class="btcpay-theme-switch-label">Theme</span> + <span class="btcpay-theme-switch-label" id="theme-switch-label">Theme</span> - <div class="btcpay-theme-switch-themes"> - <button type="button" title="System" data-theme="system"><vc:icon symbol="themes-system"/></button> - <button type="button" title="Light" data-theme="light"><vc:icon symbol="themes-light"/></button> - <button type="button" title="Dark" data-theme="dark"><vc:icon symbol="themes-dark"/></button> + <div class="btcpay-theme-switch-themes" role="group" aria-labelledby="theme-switch-label"> + <button type="button" title="System" data-theme="system" aria-label="System theme"><vc:icon symbol="themes-system"/></button> + <button type="button" title="Light" data-theme="light" aria-label="Light theme"><vc:icon symbol="themes-light"/></button> + <button type="button" title="Dark" data-theme="dark" aria-label="Dark theme"><vc:icon symbol="themes-dark"/></button> </div> </div> </li>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PluginBuilder/Components/MainNav/Default.cshtml` around lines 172 - 181, The theme-toggle buttons in the btcpay-theme-switch-themes div (buttons with data-theme attributes and <vc:icon> children) need ARIA attributes: add an aria-label (e.g., "Set theme to {system|light|dark}") to each button and include an aria-pressed attribute that reflects the active state; ensure the container has an appropriate grouping role (e.g., role="group" or role="tablist") and update the script that handles theme changes (the code that reads/writes data-theme) to set aria-pressed="true" on the active button and aria-pressed="false" on the others so screen readers know which theme is selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PluginBuilder/Views/Shared/_CommonHead.cshtml`:
- Around line 12-18: The inline IIFE that reads localStorage 'btcpay-theme' and
toggles the dark stylesheet only handles an explicit 'dark' value, causing a
light-theme flash for users who have no stored preference but whose system
prefers dark; update the IIFE to, when t is falsy, check
window.matchMedia('(prefers-color-scheme: dark)').matches and if true set
document.documentElement.setAttribute('data-btcpay-theme','dark') and enable the
element with id 'DarkThemeLinkTag' (and only disable/enable that link based on
the resolved theme), otherwise keep existing behavior for explicit localStorage
values; reference the localStorage key 'btcpay-theme', the IIFE block, and the
DOM id 'DarkThemeLinkTag' when making this change.
---
Nitpick comments:
In `@PluginBuilder/Components/MainNav/Default.cshtml`:
- Around line 172-181: The theme-toggle buttons in the
btcpay-theme-switch-themes div (buttons with data-theme attributes and <vc:icon>
children) need ARIA attributes: add an aria-label (e.g., "Set theme to
{system|light|dark}") to each button and include an aria-pressed attribute that
reflects the active state; ensure the container has an appropriate grouping role
(e.g., role="group" or role="tablist") and update the script that handles theme
changes (the code that reads/writes data-theme) to set aria-pressed="true" on
the active button and aria-pressed="false" on the others so screen readers know
which theme is selected.
In `@PluginBuilder/wwwroot/main/layout.css`:
- Around line 233-240: Add a blank line before the .btcpay-theme-switch-themes
button rule to satisfy Stylelint formatting preference; locate the CSS selector
".btcpay-theme-switch-themes button" in layout.css and insert a single empty
line immediately above that rule so the file conforms to the project's Stylelint
spacing rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2174833f-db43-43c6-8a58-f067ef48209f
⛔ Files ignored due to path filters (1)
PluginBuilder/wwwroot/img/icon-sprite.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
PluginBuilder/Components/MainNav/Default.cshtmlPluginBuilder/Views/Home/GetPluginDetails.cshtmlPluginBuilder/Views/Shared/_CommonHead.cshtmlPluginBuilder/Views/Shared/_Layout.cshtmlPluginBuilder/Views/Shared/_PublicHeader.cshtmlPluginBuilder/wwwroot/js/theme-switch.jsPluginBuilder/wwwroot/main/layout.css
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@PluginBuilder/Views/Shared/_CommonHead.cshtml`:
- Around line 12-19: In the IIFE in _CommonHead.cshtml the local variable is
named theme but the code erroneously references t, causing a ReferenceError;
change the reference from t to theme (in the if that sets data-btcpay-theme) so
the IIFE uses the declared variable, and verify the rest of the logic around
determining isDark and toggling the element with id 'DarkThemeLinkTag' remains
the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4d4c755-bb01-4b67-aa9c-4c4ea15797d1
📒 Files selected for processing (1)
PluginBuilder/Views/Shared/_CommonHead.cshtml
Collaborator
|
Nice work @rollforsats Thank you |
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
2 participants
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.
New theme toggle matches BTCPayServer theme toggle settings
Add dark mode with auto/light/dark theme toggle
directory.mov
plugin.mov
dashboard.mov