Skip to content

feat: Keyboard shortcuts to toggle map overlays#646

Closed
KidVizious wants to merge 2 commits intoaccius:Stagingfrom
KidVizious:add-map-overlay-shortcuts
Closed

feat: Keyboard shortcuts to toggle map overlays#646
KidVizious wants to merge 2 commits intoaccius:Stagingfrom
KidVizious:add-map-overlay-shortcuts

Conversation

@KidVizious
Copy link

@KidVizious KidVizious commented Mar 1, 2026

What does this PR do?

Adds key bindings to toggle map layers on and off. It also adds both a floating panel and dockable panel with the current list of dynamically created keybinds.

Resolves #635

Type of change

  • Bug fix
  • New feature
  • Performance improvement
  • Refactor / code cleanup
  • Documentation
  • Translation
  • Map layer plugin

How to test

  1. Press "?" key, panel should appear with a list of keybindings
  2. Switch to dockable layout and add a panel, "Keyboard Shortcuts"
  3. Panels should respect user's theme choice
  4. Press each key listed to toggle the respective layer on/off

Checklist

  • App loads without console errors
  • Tested in Dark, Light, and Retro themes
  • Responsive at different screen sizes (desktop + mobile)
  • If touching server.js: caches have TTLs and size caps (we serve 2,000+ concurrent users)
  • If adding an API route: includes caching and error handling
  • If adding a panel: wired into Modern, Classic, and Dockable layouts
  • No hardcoded colors — uses CSS variables (var(--accent-cyan), etc.)
  • No .bak, .old, console.log debug lines, or test scripts included

Screenshots (if visual change)

Some sample screenshots are below. I didn't post every combo for brevity sake.

Screenshot 2026-02-28 200313 Screenshot 2026-02-28 200333 Screenshot 2026-02-28 200216

@accius
Copy link
Owner

accius commented Mar 1, 2026

Hey, nice work on this — keyboard shortcuts for layer toggling is a great addition and the dual-mode panel (modal + dockable) is well thought out. A few things I'd like addressed before merging:

Bug — Indentation
The closing /> on <KeybindingsPanel> in App.jsx (around line 531) uses a tab instead of spaces. Please fix to match the rest of the codebase.

Bug — Removed translation key
The propagation.heatmap.tooltip.stoplight key was dropped from en.json. Is that intentional? If it's still referenced anywhere that'll break. Can you grep for it and confirm?

Concern — Shortcut stability
The auto-assignment algorithm (first unique letter per layer name) is clever, but it means shortcuts silently reshuffle whenever a layer is added or renamed. A user who learns "G = Greyline" could find G suddenly toggles something else after an update. Would you be open to adding an optional shortcut property to the layer registry so we can pin important ones? The auto-assign can still be the fallback for layers that don't specify one.

Minor — Global coupling
The window.hamclockLayerControls usage works but is a bit fragile. Not a blocker for this PR, but something we should think about cleaning up down the road.

Minor — Static deps on useMemo
layerShortcuts has [] dependencies so it only computes on mount. That's fine if all layers are registered at startup, but worth noting — if we ever load layers dynamically this will need to update.

TL;DR: Fix the indentation and confirm the removed translation key, and this is good to go. The shortcut stability thing can be a follow-up issue. Thanks for putting this together!

@accius
Copy link
Owner

accius commented Mar 3, 2026

I am closing this and adding in the fixes as I wouldn't mind having this in tomorrows prod release.

@accius accius closed this Mar 3, 2026
@KidVizious
Copy link
Author

KidVizious commented Mar 3, 2026

@accius I implemented your fixes as well as some other improvements if you want to take another look.

  • Layer names now sorted before assigning keys
  • Only use layer.id instead of name. Seems like these are used sort of interchangeably between layers but id is usually more sane for keybindings
  • Stoplight was in en.json twice
  • Fixed tab indentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants