Copilot commented on code in PR #277:
URL: https://github.com/apache/shiro-site/pull/277#discussion_r2703776556
##########
src/site/assets/css/base.css:
##########
@@ -213,3 +254,153 @@ div.related-content {
.related-content .read-more {
font-size: 11px;
}
+
+/* Dark mode overrides for Bootstrap components */
+[data-theme="dark"] .navbar-light {
+ background-color: var(--navbar-bg) !important;
+}
+
+[data-theme="dark"] .navbar-light .navbar-nav .nav-link {
+ color: var(--text-color);
+}
+
+[data-theme="dark"] .navbar-light .navbar-nav .nav-link:hover {
+ color: var(--link-color);
+}
+
+[data-theme="dark"] .dropdown-menu {
+ background-color: var(--bg-secondary);
+ border-color: var(--border-color);
+}
+
+[data-theme="dark"] .dropdown-item {
+ color: var(--text-color);
+}
+
+[data-theme="dark"] .dropdown-item:hover {
+ background-color: var(--border-color);
+ color: var(--text-color);
+}
+
+[data-theme="dark"] .dropdown-divider {
+ border-color: var(--border-color);
+}
+
+[data-theme="dark"] .bg-light {
+ background-color: var(--navbar-bg) !important;
+}
+
+[data-theme="dark"] .border-top {
+ border-color: var(--border-color) !important;
+}
+
+[data-theme="dark"] .text-muted {
+ color: var(--text-muted) !important;
+}
+
+[data-theme="dark"] .shadow-sm {
+ box-shadow: 0 0.125rem 0.25rem rgba(0, 0, 0, 0.4) !important;
+}
+
+/* Code blocks dark mode */
+[data-theme="dark"] pre,
+[data-theme="dark"] code {
+ background-color: var(--code-bg);
+ color: var(--code-text);
+}
+
+[data-theme="dark"] .hljs {
+ background: var(--code-bg);
+ color: var(--code-text);
+}
+
+/* Theme toggle button */
+#theme-toggle {
+ background: none;
+ border: 1px solid var(--border-color);
+ border-radius: 4px;
+ padding: 4px 8px;
+ cursor: pointer;
+ font-size: 16px;
+ margin-left: 10px;
+ transition: background-color 0.3s ease;
+}
+
Review Comment:
The theme toggle button should include keyboard support information.
Consider adding a keyboard shortcut or documenting that it can be activated
using the keyboard. Additionally, the button should visually indicate focus
state for keyboard navigation.
```suggestion
#theme-toggle:focus,
#theme-toggle:focus-visible {
outline: 2px solid var(--link-color);
outline-offset: 2px;
background-color: var(--bg-secondary);
}
```
##########
src/site/templates/menu.ftl:
##########
@@ -85,6 +86,12 @@
<li><a class="dropdown-item"
href="https://www.apache.org/security/";>Security</a></li>
</ul>
</li>
+ <!-- Theme toggle -->
+ <li class="nav-item d-flex align-items-center">
+ <button id="theme-toggle" aria-label="Toggle dark mode"
title="Toggle dark mode">
Review Comment:
The theme toggle button lacks a type attribute. Buttons without an explicit
type default to "submit" in forms. To avoid unintended behavior, add
type="button" to the button element.
```suggestion
<button id="theme-toggle" type="button" aria-label="Toggle
dark mode" title="Toggle dark mode">
```
##########
src/site/assets/js/theme.js:
##########
@@ -0,0 +1,30 @@
+(function () {
+ var storedTheme = localStorage.getItem("theme");
+
+ var theme = storedTheme || "light";
+ document.documentElement.setAttribute("data-theme", theme);
Review Comment:
The code accesses localStorage without error handling. In private browsing
mode or when localStorage is disabled, this will throw an exception and break
the theme functionality. Consider wrapping localStorage calls in try-catch
blocks to gracefully handle these scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
