mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-10 09:19:42 +02:00
feat(settings): add an Apply button that saves without closing
Switches such as MCP server sync only unlock their controls once saved, and Save closed the modal, so the user had to reopen Settings to continue. Apply runs the same save, keeps the modal open and refreshes the dependent groups (MCP sync, custom model endpoints, CLI management) in place.
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'aicodeman': minor
|
||||
---
|
||||
|
||||
Settings has an Apply button next to Save. It saves the same way but keeps Settings open and refreshes the groups that depend on a saved value, so switching on MCP server sync (or custom model endpoints, or CLI management) brings up its controls straight away instead of needing a close-and-reopen. The MCP sync hint now says to Apply or Save first.
|
||||
@@ -1635,6 +1635,7 @@
|
||||
Save; row-reverse keeps Save to the left of it on phones. -->
|
||||
<div class="set-head-actions">
|
||||
<button class="modal-close" onclick="app.closeAppSettings()" aria-label="Close app settings">×</button>
|
||||
<button class="set-head-save set-head-apply" onclick="app.applyAppSettings()" title="Save and keep Settings open">Apply</button>
|
||||
<button class="set-head-save" onclick="app.saveAppSettings()">Save</button>
|
||||
</div>
|
||||
</div>
|
||||
@@ -3037,6 +3038,7 @@
|
||||
</div>
|
||||
<div class="form-actions set-foot">
|
||||
<button class="btn-toolbar" onclick="app.closeAppSettings()">Cancel</button>
|
||||
<button class="btn-toolbar" onclick="app.applyAppSettings()" title="Save and keep Settings open">Apply</button>
|
||||
<button class="btn-toolbar btn-primary" onclick="app.saveAppSettings()">Save</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -1225,7 +1225,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
};
|
||||
// Switched on in this modal but not saved yet: the routes would only answer "disabled".
|
||||
if (!this._mcpSyncSavedOn) {
|
||||
show('Save settings to turn MCP sync on first, then reopen Settings to preview or sync.');
|
||||
show('Apply or Save settings to turn MCP sync on first, then preview or sync.');
|
||||
return;
|
||||
}
|
||||
if (apply && !confirm('Add missing MCP servers to every installed, enabled CLI\'s config file? Env values and headers on those servers are copied too.')) return;
|
||||
@@ -2506,7 +2506,25 @@ Object.assign(CodemanApp.prototype, {
|
||||
claudeEl.className = 'voice-provider-status' + (status?.available ? ' active' : '');
|
||||
},
|
||||
|
||||
/**
|
||||
* Apply button: the same save as Save, but the modal stays open and the groups that depend on
|
||||
* a saved value (MCP sync, custom model endpoints, CLI management) are refreshed in place, so
|
||||
* a switch that unlocks more settings needs no close-and-reopen. It is a wrapper rather than
|
||||
* an option on saveAppSettings() so that function's signature (which tests locate by text)
|
||||
* stays as it was.
|
||||
*/
|
||||
async applyAppSettings() {
|
||||
if (this._applyingSettings) return;
|
||||
this._applyingSettings = true;
|
||||
try {
|
||||
await this.saveAppSettings();
|
||||
} finally {
|
||||
this._applyingSettings = false;
|
||||
}
|
||||
},
|
||||
|
||||
async saveAppSettings() {
|
||||
const keepOpen = this._applyingSettings === true;
|
||||
// Gesture overlay is injected at page render (server-side), so a change to it
|
||||
// only takes effect on reload — remember the prior value to decide below.
|
||||
const _prev = this.loadAppSettingsFromStorage();
|
||||
@@ -2866,7 +2884,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.saveAppSettingsToStorage(settings);
|
||||
const cb = document.getElementById('appSettingsTunnelEnabled');
|
||||
if (cb) cb.checked = false;
|
||||
this.closeAppSettings();
|
||||
if (!keepOpen) this.closeAppSettings();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -2881,7 +2899,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
if (webhookError) {
|
||||
this.showToast(`Settings saved, but not the webhook: ${webhookError}`, 'warning');
|
||||
} else {
|
||||
this.showToast('Settings saved', 'success');
|
||||
this.showToast(keepOpen ? 'Settings applied' : 'Settings saved', 'success');
|
||||
}
|
||||
|
||||
// Show tunnel-specific feedback if toggled on
|
||||
@@ -2895,6 +2913,8 @@ Object.assign(CodemanApp.prototype, {
|
||||
|
||||
if (webhookError) {
|
||||
document.getElementById('webhookGroup')?.scrollIntoView({ block: 'center' });
|
||||
} else if (keepOpen) {
|
||||
this._refreshSettingsAfterApply(settings);
|
||||
} else {
|
||||
this.closeAppSettings();
|
||||
}
|
||||
@@ -2916,6 +2936,25 @@ Object.assign(CodemanApp.prototype, {
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* After Apply: bring the groups whose contents depend on a SAVED value up to date without
|
||||
* reopening the modal. openAppSettings does the same on open; this is the part of it that
|
||||
* a save can change, without touching what the user is editing or the scroll position.
|
||||
*/
|
||||
_refreshSettingsAfterApply(settings) {
|
||||
// The MCP routes read the saved flag, so switching it on is only usable from now.
|
||||
this._mcpSyncSavedOn = settings.mcpSyncEnabled === true;
|
||||
const out = this.$('mcpSyncResult');
|
||||
if (this._mcpSyncSavedOn && out && out.textContent.startsWith('Apply or Save settings')) {
|
||||
out.style.display = 'none';
|
||||
out.innerHTML = '';
|
||||
}
|
||||
this.applyMcpSyncVisibility();
|
||||
this.applyCustomModelEndpointsVisibility();
|
||||
this.applyCliManagementVisibility();
|
||||
this._applyDoctorAdminGate();
|
||||
},
|
||||
|
||||
// Load model configuration from server for the settings modal
|
||||
async loadModelConfigForSettings() {
|
||||
try {
|
||||
|
||||
@@ -16974,6 +16974,12 @@ html[data-tab-orientation='vertical'] .home-sessions {
|
||||
transform 0.1s ease;
|
||||
}
|
||||
|
||||
:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save.set-head-apply {
|
||||
background: transparent;
|
||||
color: var(--text);
|
||||
box-shadow: inset 0 0 0 1px var(--border);
|
||||
}
|
||||
|
||||
:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save:hover {
|
||||
filter: brightness(1.08);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* @fileoverview Settings "Apply": saves like Save but keeps the modal open and refreshes the
|
||||
* groups that depend on a saved value (MCP sync, custom model endpoints, CLI management).
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const publicDir = resolve(import.meta.dirname, '../src/web/public');
|
||||
const html = readFileSync(resolve(publicDir, 'index.html'), 'utf8');
|
||||
const settingsUi = readFileSync(resolve(publicDir, 'settings-ui.js'), 'utf8');
|
||||
|
||||
function loadApp() {
|
||||
const CodemanApp = function CodemanApp() {} as unknown as { prototype: Record<string, unknown> };
|
||||
const context = vm.createContext({
|
||||
CodemanApp,
|
||||
localStorage: { getItem: () => null, setItem: () => {} },
|
||||
document: { getElementById: () => null, querySelectorAll: () => [], addEventListener: () => {} },
|
||||
window: {},
|
||||
console,
|
||||
});
|
||||
vm.runInContext(settingsUi, context, { filename: 'settings-ui.js' });
|
||||
return new (CodemanApp as unknown as new () => Record<string, any>)();
|
||||
}
|
||||
|
||||
describe('Settings Apply button', () => {
|
||||
it('sits next to Save in the footer and the phone header, wired to applyAppSettings', () => {
|
||||
const start = html.indexOf('<div class="modal" id="appSettingsModal">');
|
||||
const modal = html.slice(start, html.indexOf('<!-- Shortcut Overlay Modal -->', start));
|
||||
expect(modal.match(/onclick="app\.applyAppSettings\(\)"/g)).toHaveLength(2);
|
||||
expect(modal.match(/onclick="app\.saveAppSettings\(\)"/g)).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('flags the save as keep-open while it runs and clears the flag afterwards', async () => {
|
||||
const app = loadApp();
|
||||
const seen: boolean[] = [];
|
||||
app.saveAppSettings = vi.fn(async () => {
|
||||
seen.push(app._applyingSettings);
|
||||
});
|
||||
await app.applyAppSettings();
|
||||
expect(seen).toEqual([true]);
|
||||
expect(app._applyingSettings).toBe(false);
|
||||
});
|
||||
|
||||
it('clears the flag even when the save throws, and ignores a second click mid-save', async () => {
|
||||
const app = loadApp();
|
||||
let release!: () => void;
|
||||
app.saveAppSettings = vi.fn(() => new Promise<void>((r) => (release = r)));
|
||||
const first = app.applyAppSettings();
|
||||
await app.applyAppSettings();
|
||||
expect(app.saveAppSettings).toHaveBeenCalledTimes(1);
|
||||
release();
|
||||
await first;
|
||||
|
||||
app.saveAppSettings = vi.fn(async () => {
|
||||
throw new Error('boom');
|
||||
});
|
||||
await expect(app.applyAppSettings()).rejects.toThrow('boom');
|
||||
expect(app._applyingSettings).toBe(false);
|
||||
});
|
||||
|
||||
it('refreshes the dependent groups from the saved values', () => {
|
||||
const app = loadApp();
|
||||
const out = {
|
||||
textContent: 'Apply or Save settings to turn MCP sync on first',
|
||||
style: { display: 'block' },
|
||||
innerHTML: 'x',
|
||||
};
|
||||
app.$ = (id: string) => (id === 'mcpSyncResult' ? out : null);
|
||||
app.applyMcpSyncVisibility = vi.fn();
|
||||
app.applyCustomModelEndpointsVisibility = vi.fn();
|
||||
app.applyCliManagementVisibility = vi.fn();
|
||||
app._applyDoctorAdminGate = vi.fn();
|
||||
app._mcpSyncSavedOn = false;
|
||||
|
||||
app._refreshSettingsAfterApply({ mcpSyncEnabled: true });
|
||||
|
||||
expect(app._mcpSyncSavedOn).toBe(true);
|
||||
expect(out.style.display).toBe('none');
|
||||
expect(app.applyMcpSyncVisibility).toHaveBeenCalled();
|
||||
expect(app.applyCustomModelEndpointsVisibility).toHaveBeenCalled();
|
||||
expect(app.applyCliManagementVisibility).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('only closes the modal on a plain Save', () => {
|
||||
const body = settingsUi.slice(settingsUi.indexOf('\n async saveAppSettings() {'));
|
||||
expect(body).toContain('const keepOpen = this._applyingSettings === true;');
|
||||
expect(body).toMatch(
|
||||
/else if \(keepOpen\) \{\s*this\._refreshSettingsAfterApply\(settings\);\s*\} else \{\s*this\.closeAppSettings\(\);/
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user