fix(review): preserve shortcutOverrides across settings saves + keep help modal reachable (PR #146)

- saveAppSettings() rebuilds settings from the DOM; carry over shortcutOverrides
  like showTokenCount/showCost so rebinding survives unrelated saves
- shortcut overlay footer links to the full help modal (its only opener was the
  legacy Ctrl+? route this PR replaced)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-07-12 19:58:32 +02:00
parent 88f47754ad
commit a8e7669f5a
4 changed files with 29 additions and 0 deletions
+3
View File
@@ -1679,6 +1679,9 @@
</div>
<div class="modal-body">
<div id="shortcutOverlayList"></div>
<div class="shortcut-overlay-footer">
<button class="btn btn-sm" onclick="app.closeShortcutOverlay(); app.showHelp()">Full shortcut reference</button>
</div>
</div>
</div>
</div>
+3
View File
@@ -1458,6 +1458,9 @@ Object.assign(CodemanApp.prototype, {
// with no UI left to turn it back off. Preserve the prior stored preference.
if (_prev.showTokenCount !== undefined) settings.showTokenCount = _prev.showTokenCount;
if (_prev.showCost !== undefined) settings.showCost = _prev.showCost;
// Shortcut overrides are edited from the Shortcuts tab (not rebuilt from the
// general-settings DOM), so the fresh rebuild would drop them on every save.
if (_prev.shortcutOverrides !== undefined) settings.shortcutOverrides = _prev.shortcutOverrides;
// Save to localStorage
this.saveAppSettingsToStorage(settings);
+7
View File
@@ -5812,6 +5812,13 @@ kbd {
color: var(--text-dim);
}
.shortcut-overlay-footer {
margin-top: 0.75rem;
padding-top: 0.75rem;
border-top: 1px solid var(--border);
text-align: right;
}
.away-digest-ranges {
display: flex;
flex-wrap: wrap;
+16
View File
@@ -59,6 +59,22 @@ describe('shortcut registry and overlay', () => {
expect(css).toContain('.shortcut-capture-btn,');
expect(css).toContain('.shortcut-overlay-row {');
});
it('saveAppSettings preserves shortcutOverrides (rebuilt-from-DOM saves must not wipe them)', () => {
// Same trap as showTokenCount/showCost: saveAppSettings() rebuilds the settings
// object fresh from the DOM, so keys edited elsewhere (the Shortcuts tab) must be
// explicitly carried over from the previously stored blob.
expect(settingsSource).toContain(
'if (_prev.shortcutOverrides !== undefined) settings.shortcutOverrides = _prev.shortcutOverrides;'
);
});
it('keeps the full help modal reachable now that Ctrl+? opens the registry overlay', () => {
// The legacy #helpModal (full shortcut reference) lost its only opener when
// Ctrl+? was rerouted to the overlay; the overlay footer must link to it.
expect(htmlSource).toContain('shortcut-overlay-footer');
expect(htmlSource).toContain('app.closeShortcutOverlay(); app.showHelp()');
});
});
// ─── Functional coverage (vm-sandbox harness, mirrors run-mode-ui.test.ts) ────