Accessibility, Visual Customization, Player Experience, Mobile Support and CLI Configuration#40
Conversation
…olorblind & settings CSS Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
… auto-play toggle, colorblind modes, settings panel, game-over banner, stat color-coding, and policy progress bar Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/875a2b7b-2e63-4ea1-bdb0-98125ff3d0d6 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…, SettingsPanel, ThemeManager with colorblind modes and tests Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/875a2b7b-2e63-4ea1-bdb0-98125ff3d0d6 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…orblind modes, and new keybindings Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/875a2b7b-2e63-4ea1-bdb0-98125ff3d0d6 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes player-facing feedback and accessibility controls across the web and desktop clients by replacing blocking modal notifications with toast-style messaging and introducing theme/colorblind/font scaling options plus richer hover/selection UI.
Changes:
- Web: adds toast notifications, canvas hover tooltips + selection ring, autoplay toggle, policy cooldown progress UI, and a settings panel persisted to
localStorage. - Desktop: introduces
NotificationManager+ OpenGL overlays (toast, tooltip, settings, notification log) and aThemeManagerpersisted to~/.barony/settings.json. - Adds JUnit coverage for the new desktop notification/theme managers and updates user docs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| web-client/src/main/resources/templates/game.html | Adds web toasts, settings persistence, canvas tooltips/selection, autoplay toggle, and HUD improvements. |
| web-client/src/main/resources/static/css/style.css | Styles for toasts/tooltips/banners/settings; introduces theme/colorblind/font CSS custom properties. |
| frontend/src/test/java/com/barony/frontend/ui/NotificationManagerTest.java | Unit tests for NotificationManager behavior (capacity, severity, critical persistence). |
| frontend/src/test/java/com/barony/frontend/rendering/ThemeManagerTest.java | Unit tests for ThemeManager palette/theme switching and font scale clamping. |
| frontend/src/main/resources/settings/default-settings.json | Adds a default settings JSON resource. |
| frontend/src/main/java/com/barony/frontend/ui/TooltipOverlay.java | New OpenGL tooltip renderer (cursor-adjacent, wrapped text). |
| frontend/src/main/java/com/barony/frontend/ui/ToastOverlay.java | New OpenGL toast renderer reading from NotificationManager. |
| frontend/src/main/java/com/barony/frontend/ui/SettingsPanel.java | New in-game settings panel (F9) that applies ThemeManager settings and saves them. |
| frontend/src/main/java/com/barony/frontend/ui/NotificationManager.java | New singleton manager for toast lifecycle + history buffer. |
| frontend/src/main/java/com/barony/frontend/ui/NotificationLogPanel.java | New history panel renderer for past notifications. |
| frontend/src/main/java/com/barony/frontend/rendering/ThemeManager.java | New centralized theme/colorblind/font scale manager with JSON persistence. |
| frontend/src/main/java/com/barony/frontend/FrontendApplication.java | Wires in ThemeManager-based army colors and renders overlays; adds F9 settings handling. |
| README.md | Documents new settings keybind/features and web client enhancements. |
| PLAYER_GUIDE.md | Adds accessibility/settings section and updates controls/help text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| glColor4f(0.54f, 0.45f, 0.33f, 1.0f); | ||
| glLineWidth(2.0f); | ||
| glBegin(GL_LINE_LOOP); | ||
| glVertex2f(PANEL_X, PANEL_Y); | ||
| glVertex2f(PANEL_X + PANEL_WIDTH, PANEL_Y); | ||
| glVertex2f(PANEL_X + PANEL_WIDTH, PANEL_Y + PANEL_HEIGHT); | ||
| glVertex2f(PANEL_X, PANEL_Y + PANEL_HEIGHT); | ||
| glEnd(); |
There was a problem hiding this comment.
NotificationLogPanel changes global OpenGL line width (glLineWidth(2.0f)) but does not restore it before returning. Since GL state persists across frames, this can unintentionally affect other rendering. Consider resetting line width to 1.0f before glPopAttrib() or pushing/restoring GL_LINE_BIT as well.
| glColor4f(0.79f, 0.66f, 0.43f, 1.0f); | ||
| glLineWidth(2.0f); | ||
| glBegin(GL_LINE_LOOP); | ||
| glVertex2f(PANEL_X, PANEL_Y); | ||
| glVertex2f(PANEL_X + PANEL_WIDTH, PANEL_Y); | ||
| glVertex2f(PANEL_X + PANEL_WIDTH, PANEL_Y + PANEL_HEIGHT); | ||
| glVertex2f(PANEL_X, PANEL_Y + PANEL_HEIGHT); | ||
| glEnd(); |
There was a problem hiding this comment.
SettingsPanel changes global OpenGL line width (glLineWidth(2.0f)) but does not restore it before returning. Since GL state persists across frames, this can unintentionally affect other rendering. Consider resetting line width to 1.0f before glPopAttrib() or pushing/restoring GL_LINE_BIT as well.
| // Overlay UI components | ||
| private ToastOverlay toastOverlay = new ToastOverlay(); | ||
| private TooltipOverlay tooltipOverlay = new TooltipOverlay(); | ||
| private NotificationLogPanel notificationLogPanel = new NotificationLogPanel(); | ||
| private SettingsPanel settingsPanel = new SettingsPanel(); |
There was a problem hiding this comment.
FrontendApplication declares a TooltipOverlay instance but it is never set/updated or rendered, so desktop tooltips won’t appear even though the overlay exists. Either wire it into hover handling (set/hide on hover changes) and render it with the other overlays, or remove the unused field/import to avoid confusion.
| if (settingsPanel.isVisible()) { | ||
| settingsPanel.hide(); | ||
| } else if (notificationLogPanel.isVisible()) { | ||
| notificationLogPanel.hide(); | ||
| } else if (splitModeActive) { | ||
| splitModeActive = false; | ||
| splitModeArmyId = -1; | ||
| splitModeTotalSoldiers = 0; | ||
| System.out.println("Split mode cancelled"); | ||
| NotificationManager.getInstance().info("Split mode cancelled"); | ||
| } else { | ||
| glfwSetWindowShouldClose(window, true); | ||
| } | ||
| } | ||
|
|
||
| // F9 toggles settings panel | ||
| if (key == GLFW_KEY_F9 && action == GLFW_RELEASE) { | ||
| settingsPanel.toggle(); | ||
| return; | ||
| } | ||
|
|
||
| // Settings panel input handling | ||
| if (settingsPanel.isVisible() && action == GLFW_RELEASE) { | ||
| if (key == GLFW_KEY_UP) settingsPanel.navigateUp(); | ||
| else if (key == GLFW_KEY_DOWN) settingsPanel.navigateDown(); | ||
| else if (key == GLFW_KEY_LEFT) settingsPanel.navigateLeft(); | ||
| else if (key == GLFW_KEY_RIGHT) settingsPanel.navigateRight(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
NotificationLogPanel is rendered and can be closed via ESC, but there is no keybind/menu action to ever open it (no calls to notificationLogPanel.toggle()). As a result, the log panel is effectively unreachable. Consider adding a dedicated keybind (and optional scroll key handling while visible) or removing the panel integration if it’s not meant to be user-accessible yet.
| "colorblindMode": "none", | ||
| "theme": "dark", | ||
| "fontScale": 1.0 | ||
| } |
There was a problem hiding this comment.
This default settings JSON file doesn't appear to be referenced/loaded anywhere in the codebase, so it will be dead weight in the build artifacts. Either wire it into ThemeManager (e.g., use it when ~/.barony/settings.json is missing) or remove it to avoid confusion about which defaults are authoritative.
| "colorblindMode": "none", | |
| "theme": "dark", | |
| "fontScale": 1.0 | |
| } | |
| "_comment": "Deprecated: this file is not used by ThemeManager or the runtime. Defaults are defined in code and/or user settings." | |
| } |
| ### Web Client Features | ||
| - Toast notifications replace all blocking alerts | ||
| - Canvas hover tooltips (tile info, army stats, castle capture progress) |
There was a problem hiding this comment.
PR description/README says all blocking alert() calls were replaced in the web client, but web-client/src/main/resources/templates/login.html still contains alert('Please enter a username') and alert('Login failed: ...'). Either migrate login.html to the toast/non-blocking pattern as well, or narrow the claim/docs to only the in-game page.
| themeName = (String) settings.get("theme"); | ||
| } | ||
| if (settings.containsKey("fontScale")) { | ||
| fontScale = ((Number) settings.get("fontScale")).floatValue(); |
There was a problem hiding this comment.
ThemeManager.load() assigns fontScale directly from disk without clamping. A corrupted/hand-edited settings.json could set an invalid scale and break layout. Consider routing the loaded value through setFontScale() (or applying the same clamp logic) before storing it.
| fontScale = ((Number) settings.get("fontScale")).floatValue(); | |
| setFontScale(((Number) settings.get("fontScale")).floatValue()); |
| private void loadFromThemeManager() { | ||
| ThemeManager tm = ThemeManager.getInstance(); | ||
| String cb = tm.getColorblindMode(); | ||
| for (int i = 0; i < COLORBLIND_OPTIONS.length; i++) { | ||
| if (COLORBLIND_OPTIONS[i].equalsIgnoreCase(cb)) { colorblindIndex = i; break; } | ||
| } | ||
| String theme = tm.getThemeName(); | ||
| for (int i = 0; i < THEME_OPTIONS.length; i++) { | ||
| if (THEME_OPTIONS[i].equalsIgnoreCase(theme)) { themeIndex = i; break; } |
There was a problem hiding this comment.
SettingsPanel.loadFromThemeManager() matches theme names via equalsIgnoreCase against "High Contrast". If ThemeManager ever persists/returns "high-contrast" (hyphen) instead of "high contrast" (space), this will fail to match and the panel will show the wrong selected theme. Consider normalizing both strings (e.g., replace '-' with ' ' before comparison) or using a canonical enum/value map.
| private void loadFromThemeManager() { | |
| ThemeManager tm = ThemeManager.getInstance(); | |
| String cb = tm.getColorblindMode(); | |
| for (int i = 0; i < COLORBLIND_OPTIONS.length; i++) { | |
| if (COLORBLIND_OPTIONS[i].equalsIgnoreCase(cb)) { colorblindIndex = i; break; } | |
| } | |
| String theme = tm.getThemeName(); | |
| for (int i = 0; i < THEME_OPTIONS.length; i++) { | |
| if (THEME_OPTIONS[i].equalsIgnoreCase(theme)) { themeIndex = i; break; } | |
| /** | |
| * Normalize a theme name for comparison. | |
| * Currently replaces hyphens with spaces and trims whitespace. | |
| */ | |
| private String normalizeThemeName(String name) { | |
| if (name == null) { | |
| return ""; | |
| } | |
| return name.replace('-', ' ').trim(); | |
| } | |
| private void loadFromThemeManager() { | |
| ThemeManager tm = ThemeManager.getInstance(); | |
| String cb = tm.getColorblindMode(); | |
| for (int i = 0; i < COLORBLIND_OPTIONS.length; i++) { | |
| if (COLORBLIND_OPTIONS[i].equalsIgnoreCase(cb)) { colorblindIndex = i; break; } | |
| } | |
| String theme = normalizeThemeName(tm.getThemeName()); | |
| for (int i = 0; i < THEME_OPTIONS.length; i++) { | |
| String option = normalizeThemeName(THEME_OPTIONS[i]); | |
| if (option.equalsIgnoreCase(theme)) { themeIndex = i; break; } |
| float bottomStart = -1.0f + TOAST_BOTTOM_MARGIN; | ||
|
|
||
| for (int i = 0; i < notifications.length; i++) { | ||
| Notification n = notifications[i]; |
There was a problem hiding this comment.
ToastOverlay renders notifications in the order returned by NotificationManager (oldest→newest), which places the newest toast furthest from the bottom edge. This is the opposite of the web client (newest closest to bottom). Consider iterating in reverse (or reversing the snapshot) so newly created notifications appear at the bottom of the stack.
| Notification n = notifications[i]; | |
| // Render newest notifications closest to the bottom edge | |
| Notification n = notifications[notifications.length - 1 - i]; |
|
@copilot address comments |
…, add aria attrs, wire CSS vars, clamp fontScale on load, add F10 keybind for log panel, remove unused default-settings.json Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/897c4d45-112c-4485-8cdc-43a77c9884dc Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All 12 review comments addressed in
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| canvas.addEventListener('mousemove', function(e) { | ||
| if (!lastGameState) return; | ||
|
|
||
| const rect = canvas.getBoundingClientRect(); | ||
| const x = e.clientX - rect.left; | ||
| const y = e.clientY - rect.top; | ||
| const cellWidth = canvas.width / lastGameState.width; | ||
| const cellHeight = canvas.height / lastGameState.height; | ||
| const gridX = Math.floor(x / cellWidth); | ||
| const gridY = Math.floor(y / cellHeight); | ||
|
|
||
| const text = getTooltipText(gridX, gridY, lastGameState); | ||
| if (text) { | ||
| tooltip.textContent = text; | ||
| tooltip.style.display = 'block'; | ||
| tooltip.style.left = (e.clientX - rect.left + 12) + 'px'; | ||
| tooltip.style.top = (e.clientY - rect.top - 8) + 'px'; | ||
| } else { | ||
| tooltip.style.display = 'none'; | ||
| } | ||
| }); | ||
|
|
||
| canvas.addEventListener('mouseleave', function() { | ||
| tooltip.style.display = 'none'; |
There was a problem hiding this comment.
mousemove handler calls getTooltipText() on every mousemove, and getTooltipText() does a linear search over gameState.armies via .find(). This can become a hot path and cause jank as army counts grow. Consider throttling the handler (e.g., requestAnimationFrame) and/or maintaining a position->army lookup map when updating lastGameState.
| canvas.addEventListener('mousemove', function(e) { | |
| if (!lastGameState) return; | |
| const rect = canvas.getBoundingClientRect(); | |
| const x = e.clientX - rect.left; | |
| const y = e.clientY - rect.top; | |
| const cellWidth = canvas.width / lastGameState.width; | |
| const cellHeight = canvas.height / lastGameState.height; | |
| const gridX = Math.floor(x / cellWidth); | |
| const gridY = Math.floor(y / cellHeight); | |
| const text = getTooltipText(gridX, gridY, lastGameState); | |
| if (text) { | |
| tooltip.textContent = text; | |
| tooltip.style.display = 'block'; | |
| tooltip.style.left = (e.clientX - rect.left + 12) + 'px'; | |
| tooltip.style.top = (e.clientY - rect.top - 8) + 'px'; | |
| } else { | |
| tooltip.style.display = 'none'; | |
| } | |
| }); | |
| canvas.addEventListener('mouseleave', function() { | |
| tooltip.style.display = 'none'; | |
| let latestTooltipMousePosition = null; | |
| let tooltipFramePending = false; | |
| function hideTooltip() { | |
| tooltip.style.display = 'none'; | |
| } | |
| function scheduleTooltipUpdate() { | |
| if (tooltipFramePending) return; | |
| tooltipFramePending = true; | |
| requestAnimationFrame(function() { | |
| tooltipFramePending = false; | |
| if (!lastGameState || !latestTooltipMousePosition) { | |
| hideTooltip(); | |
| return; | |
| } | |
| const rect = canvas.getBoundingClientRect(); | |
| const x = latestTooltipMousePosition.clientX - rect.left; | |
| const y = latestTooltipMousePosition.clientY - rect.top; | |
| const cellWidth = canvas.width / lastGameState.width; | |
| const cellHeight = canvas.height / lastGameState.height; | |
| const gridX = Math.floor(x / cellWidth); | |
| const gridY = Math.floor(y / cellHeight); | |
| const text = getTooltipText(gridX, gridY, lastGameState); | |
| if (text) { | |
| tooltip.textContent = text; | |
| tooltip.style.display = 'block'; | |
| tooltip.style.left = (x + 12) + 'px'; | |
| tooltip.style.top = (y - 8) + 'px'; | |
| } else { | |
| hideTooltip(); | |
| } | |
| }); | |
| } | |
| canvas.addEventListener('mousemove', function(e) { | |
| latestTooltipMousePosition = { | |
| clientX: e.clientX, | |
| clientY: e.clientY | |
| }; | |
| scheduleTooltipUpdate(); | |
| }); | |
| canvas.addEventListener('mouseleave', function() { | |
| latestTooltipMousePosition = null; | |
| hideTooltip(); |
| .settings-panel { | ||
| background: #352f28; | ||
| padding: 20px; | ||
| border-radius: 8px; | ||
| margin-bottom: 20px; | ||
| border-left: 4px solid #c9a96e; | ||
| } | ||
|
|
||
| .settings-panel h3 { | ||
| margin-top: 0; | ||
| } | ||
|
|
||
| .settings-row { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 12px; | ||
| margin-bottom: 10px; | ||
| } | ||
|
|
||
| .settings-row label { | ||
| min-width: 120px; | ||
| font-weight: bold; | ||
| } | ||
|
|
||
| .settings-row select { | ||
| padding: 6px 10px; | ||
| border: 1px solid #5a4e3c; | ||
| border-radius: 4px; | ||
| background: #2b2520; | ||
| color: #d4cfc8; | ||
| font-size: 14px; |
There was a problem hiding this comment.
The Settings panel is both .info-section and .settings-panel, but .settings-panel overrides background/border colors with hard-coded values, which prevents the new theme switching (CSS custom properties) from affecting the settings UI. Use the existing CSS variables (e.g., --bg-secondary/--accent/--text-primary) for these rules so the settings panel live-preview matches the selected theme.
| public void show(String message, Severity severity) { | ||
| boolean isCritical = severity == Severity.DANGER && | ||
| (message.toLowerCase().contains("game over") || | ||
| message.toLowerCase().contains("castle") || | ||
| message.toLowerCase().contains("destroyed")); | ||
|
|
||
| Notification n = new Notification(message, severity, !isCritical); |
There was a problem hiding this comment.
NotificationManager.show() assumes message is non-null and calls message.toLowerCase(), which will throw if any code logs a null message. Also, severity can be passed as null (e.g., from future call sites), which would later break switch statements in renderers. Add null-handling (default severity to INFO, default/skip null messages) or enforce non-null inputs with a clear exception.
| public void load() { | ||
| try { | ||
| Path path = getSettingsPath(); | ||
| if (Files.exists(path)) { | ||
| String json = Files.readString(path); | ||
| Gson gson = new Gson(); | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> settings = gson.fromJson(json, Map.class); | ||
| if (settings.containsKey("colorblindMode")) { | ||
| colorblindMode = (String) settings.get("colorblindMode"); | ||
| } | ||
| if (settings.containsKey("theme")) { | ||
| themeName = (String) settings.get("theme"); | ||
| } | ||
| if (settings.containsKey("fontScale")) { | ||
| setFontScale(((Number) settings.get("fontScale")).floatValue()); | ||
| } | ||
| applyTheme(); | ||
| applyColorblindPalette(); | ||
| } |
There was a problem hiding this comment.
ThemeManager.load() assigns themeName/colorblindMode directly from JSON. If the settings file contains nulls or unexpected types, applyTheme()/applyColorblindPalette() will NPE on themeName.toLowerCase() / colorblindMode.toLowerCase(). Sanitize loaded values (default to "dark"/"none" when missing or invalid) before calling applyTheme/applyColorblindPalette.
| private void applySettings() { | ||
| ThemeManager tm = ThemeManager.getInstance(); | ||
| tm.setColorblindMode(COLORBLIND_OPTIONS[colorblindIndex].toLowerCase()); | ||
| tm.setTheme(THEME_OPTIONS[themeIndex].toLowerCase()); | ||
|
|
||
| float scale; | ||
| switch (fontSizeIndex) { | ||
| case 0: scale = 0.8f; break; | ||
| case 2: scale = 1.2f; break; | ||
| default: scale = 1.0f; break; | ||
| } | ||
| tm.setFontScale(scale); | ||
| tm.save(); | ||
| } |
There was a problem hiding this comment.
SettingsPanel.applySettings() calls ThemeManager.save() on every left/right adjustment. This can generate frequent disk writes (especially if a key repeats) and may cause stutter on slower disks. Consider saving only when closing the panel, or debounce saves (e.g., save after N ms of inactivity).
|
@copilot address comments |
…ng, throttle mousemove, theme CSS vars in settings panel, save-on-close Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/1d95564b-bd27-4c20-96f6-9497eb2013db Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All 7 review comments addressed in
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String json = Files.readString(path); | ||
| Gson gson = new Gson(); | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> settings = gson.fromJson(json, Map.class); | ||
| Object cbValue = settings.get("colorblindMode"); | ||
| if (cbValue instanceof String && !((String) cbValue).isEmpty()) { | ||
| colorblindMode = (String) cbValue; | ||
| } | ||
| Object themeValue = settings.get("theme"); | ||
| if (themeValue instanceof String && !((String) themeValue).isEmpty()) { | ||
| themeName = (String) themeValue; | ||
| } | ||
| Object scaleValue = settings.get("fontScale"); | ||
| if (scaleValue instanceof Number) { | ||
| setFontScale(((Number) scaleValue).floatValue()); | ||
| } | ||
| applyTheme(); | ||
| applyColorblindPalette(); |
There was a problem hiding this comment.
load() assigns themeName/colorblindMode directly from JSON and then dereferences settings without a null-check. gson.fromJson() can return null (e.g., empty file), causing an NPE that is swallowed by the broad catch; also, direct assignment bypasses the validation/normalization in setTheme()/setColorblindMode(). Consider guarding settings == null and applying loaded values via the setters so invalid/unknown values can’t leak into getThemeName()/getColorblindMode().
| private float[] getBackgroundColor(Severity severity) { | ||
| ThemeManager theme = ThemeManager.getInstance(); | ||
| switch (severity) { |
There was a problem hiding this comment.
getBackgroundColor() creates a ThemeManager theme = ThemeManager.getInstance(); local that is never used. This is dead code and may trigger warnings; removing it will keep the method minimal and clearer.
| // Color based on player (uses ThemeManager for colorblind support) | ||
| float[] p1Color = ThemeManager.getInstance().getPlayer1Color(); | ||
| float[] p2Color = ThemeManager.getInstance().getPlayer2Color(); | ||
| if (army.getPlayerId() == 1) { | ||
| glColor3f(0.0f, 0.0f, 1.0f); // Blue | ||
| glColor3f(p1Color[0], p1Color[1], p1Color[2]); | ||
| } else { | ||
| glColor3f(1.0f, 0.0f, 0.0f); // Red | ||
| glColor3f(p2Color[0], p2Color[1], p2Color[2]); |
There was a problem hiding this comment.
p1Color/p2Color are retrieved from ThemeManager.getInstance() for every army render. Since these colors are constant per frame, fetch them once before iterating armies to avoid repeated singleton lookups/array reads in the inner loop.
|
@copilot address comments |
…dation, dead code, color hoist Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/d4577203-1af4-477e-baf9-ba1f2f0b45c7 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All 4 review comments addressed in
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (e.key === ' ') e.preventDefault(); // prevent scroll on Space regardless | ||
| if (e.key === 'Enter' || e.key === ' ') dismissToast(toast); | ||
| }); | ||
| container.appendChild(toast); |
There was a problem hiding this comment.
#toast-container is styled with flex-direction: column-reverse (style.css:387), but new toasts are added via appendChild(). With column-reverse, appending causes newer toasts to render above older ones (oldest stays closest to the bottom), which is the opposite of the intended “newest closest to bottom” stacking. Consider either switching the container to flex-direction: column or inserting new toasts with prepend()/insertBefore() so the visual order matches the intended stacking behavior.
| container.appendChild(toast); | |
| container.prepend(toast); |
|
@copilot address comments |
…st allowlists Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/e04f4702-4dbf-43c6-89d1-7f16a8e4bc54 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Both review comments addressed in
|
|
@copilot The popup notifications in the bottom right of the desktop application can't be read, the text is too small. Also it appears to be duplication the game log. Finally, it should be possible to configure the target backend server address when runninng the desktop application from the Windows command line. Screenshot: |
… flag Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/7766f524-3ac3-48fd-b988-a9d5cf8083ab Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
All 3 issues addressed in
|
README.md
Outdated
| # Or via the start script with arguments | ||
| java -jar frontend.jar --server http://myserver:8080 |
There was a problem hiding this comment.
This doesn't work, I run into the following:
no main manifest attribute, in .\frontend-1.0.0.jar
There was a problem hiding this comment.
Fixed in 363043a. The project doesn't produce an executable jar — it runs via Maven's exec:java. Removed the misleading java -jar example and replaced with the correct mvnw compile exec:java -Dexec.args="--server ..." command for both Linux/macOS and Windows.
|
|
||
| ```bash | ||
| # Connect to a custom backend server | ||
| cd frontend && ./mvnw compile exec:java -Dexec.args="--server http://192.168.1.100:8080" |
There was a problem hiding this comment.
Document Windows equivalent here
There was a problem hiding this comment.
Added in 363043a. Both README.md and PLAYER_GUIDE.md now show the Windows equivalent:
cd frontend && mvnw.cmd compile exec:java -Dexec.args="--server http://192.168.1.100:8080"…d equivalent Agent-Logs-Url: https://github.com/Preponderous-Software/barony/sessions/572f4344-5204-402c-9cc1-3413de8fbae7 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
dmccoystephenson
changed the title
Replace all in-game blocking
alert()calls and modal notifications with a non-blocking toast system across both clients. Add canvas hover tooltips, selection feedback, colorblind modes, theme switching, font scaling, a mobile-friendly responsive layout, and CLI server configuration for the desktop client.Web Client (
game.html+style.css)showToast(message, severity)with 4 levels (INFO/SUCCESS/WARNING/DANGER), auto-dismiss 4s, max 4 stacked, critical events persist. Severity is normalized (lowercase + allowlist validation, defaults unknown values toinfo). All in-gamealert()calls replaced. Toast container includesrole="status"andaria-live="polite"for screen reader accessibility. Individual toasts are keyboard-focusable (tabindex="0") witharia-labeland Enter/Space keydown handlers so keyboard-only users can dismiss persistent critical toasts. A@media (prefers-reduced-motion: reduce)rule disables slide-in, pulse, and fade-out animations for users who request reduced motion.#FFD700) on selected army, right-click deselect,mousemovetooltip DOM div showing tile type/ownership/army stats/capture progress (throttled viarequestAnimationFrame). On touch devices, atouchstarthandler shows the same tooltip on tap with 3-second auto-dismiss, positioned below the finger to avoid obscuring content.eventToGrid()helper computes a scale ratio between the canvas pixel buffer (canvas.width) and CSS display size (getBoundingClientRect().width) so click/tap and tooltip handlers map to the correct grid cell even when the canvas is CSS-scaled on mobile.setCustomValidity()replaces alert-based validationPALETTEobject swapped by colorblind mode, CSS custom properties consumed viavar()in body, container, headings, and info-section rules for live theme/font-size switching, settings persisted tolocalStoragekeybarony_settings@media (max-width: 768px)breakpoint that stacks HUD elements vertically, wraps policy controls and settings rows for touch usability, enforces 44×44px minimum tap targets on buttons/selects, positions toasts full-width at the bottom with no clipping. Tooltip is restyled for compact display on small screens (max-width: 240px,font-size: 12px). All mobile rules use existing CSS custom properties — no hard-coded color/font values introduced.resizeCanvas()function dynamically computes canvas pixel dimensions on mobile based on the game grid's aspect ratio and available viewport space, capping height at 50% viewport height so the board doesn't dominate the screen. Cells remain square regardless of grid dimensions (e.g., a 15×13 board no longer stretches into a fixed 800×600 rectangle). On desktop the canvas stays at 800×600. A debounced (150ms)window.resizelistener triggers redraw on orientation change or resize..game-layoutflex container with CSSorderproperties places the game board and turn controls at the top of the page so the canvas is the first thing visible. Info panels (Game Status & Stats, Change Policy, Settings, Armies) are positioned below the board.<details>/<summary>elements with styled toggle indicators (▸ arrow). On mobile, "Game Status & Stats" and "Armies" default open while "Change Policy" and "Settings" default collapsed to minimize scrolling. On desktop, all sections are auto-expanded via JavaScript. Sections use semantic class names (section-status,section-policy,section-settings,section-armies) for maintainable CSS ordering.Web Client (
login.html)@media (max-width: 768px)breakpoint centers the login form vertically, constrains max-width to 90vw, increases input/button tap targets to 44px minimum height, and uses responsive font sizing. The login page is usable at 375px viewport width with no overflow or clipped elements.Desktop Frontend (Java)
NotificationManager— Singleton FIFO queue, 4 severity levels, auto-dismiss, history buffer, null-safeshow()(skips null messages, defaults null severity to INFO). Toast notifications are scoped to important game events only (turn advancement, game over, policy changes) — routine game log entries are not duplicated into toasts.ToastOverlay/NotificationLogPanel— OpenGL overlay renderers; toast renders at highest Z-order (after settings/log panels) with newest-first layout (closest to bottom edge) and enlarged readable text (2.5× scale with wider toast boxes); log panel accessible via F10 with UP/DOWN scrollThemeManager— Centralized color lookups with colorblind palettes (deuteranopia/protanopia/tritanopia), 3 themes (dark/classic/high-contrast),BASE_FONT_SIZEscaling, persisted to~/.barony/settings.jsonwith clamped fontScale on load and sanitized values against null/invalid JSON. Public setterssetTheme()/setColorblindMode()are null-safe — null/blank inputs normalize to"dark"/"none"defaults, and theme names are validated against the known set.SettingsPanel— F9 keybind, arrow-key navigation, live preview, normalized theme name matching (handles hyphen/space variants), saves only on panel close (dirty flag debounce)FrontendApplication— Wired overlays into render loop, army colors driven byThemeManager; GL line width properly restored after overlay border draws. Supports--server <url>command-line argument to configure the backend server address (defaults tohttp://localhost:8080).Tests
NotificationManager(flush, dismiss, severity, critical persistence)ThemeManager(colorblind palette swaps, theme switching, font scale clamping)Docs
README.md— F9/F10 keybinds,--serverCLI flag with platform-specific commands (Linux/macOS + Windows), web client feature list, accessibility featuresPLAYER_GUIDE.md— New "Accessibility & Visual Settings" section, updated controls table with F9/F10,--serverusage with platform-specific commands💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.