Skip to content

Commit e3c1e4d

Browse files
cansofgreaseclaude
andcommitted
Say plainly when a save did not save
Two problems on the settings footer, both of which let a refused save look like nothing had happened. A long message pushed Reset tiles onto a line of its own and slid Reset to defaults across, so the buttons jumped whenever the drawer had something to say. The message now takes the space the buttons leave and wraps inside it, which also keeps them pinned right when there is no message at all. The Save button now turns red when a click did not save, and stays that way until one does. Every refusal already explained itself beside the button, but the drawer stayed open looking untouched, which reads as a click that missed. Discarding and reopening starts clean, and the colour comes from each theme's own palette rather than a fixed red. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1028c74 commit e3c1e4d

2 files changed

Lines changed: 58 additions & 10 deletions

File tree

internal/web/ui/index.html

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,10 @@
790790
@media (max-width:600px){ .info::after{width:min(250px,86vw);left:auto;right:0;} .lbl .info::after,.ie-lbl .info::after,.netinfo .info:not(.conn-off)::after{position:fixed;left:8px !important;right:auto !important;top:auto;bottom:12px;width:min(250px,86vw);} }
791791
.drawer-actions{grid-column:1/-1;display:flex;align-items:center;flex-wrap:wrap;gap:10px 14px;
792792
border-top:1px solid var(--line);padding-top:16px;margin-top:4px;}
793+
/* The message takes whatever space the buttons leave and wraps inside it, which
794+
also keeps the buttons pinned right when it is empty. Sized to its own text it
795+
overflowed the row instead, pushing Reset tiles onto a line of its own. */
796+
#settingsMsg{flex:1 1 0;min-width:0;}
793797

794798
/* Content brightness: a black overlay above everything dims the whole page
795799
uniformly. opacity = 1 - brightness; pointer-events:none keeps the UI usable. */
@@ -1150,6 +1154,10 @@
11501154
padding:7px 14px;font:inherit;font-weight:600;font-size:12px;cursor:pointer;
11511155
transition:filter .15s,transform .08s;}
11521156
.btn:hover{filter:brightness(1.09);} .btn:active{transform:translateY(1px);} .btn:disabled{opacity:.55;cursor:default;}
1157+
/* Save goes red when a click did NOT save. The message beside it says why, but
1158+
the drawer stays open looking exactly as it did, which otherwise reads as
1159+
nothing having happened at all. It stays red until a save actually lands. */
1160+
.btn.err{background:var(--down);}
11531161
/* Busy state for ordinary buttons, same swap the RUN pill uses: the label goes
11541162
visibility:hidden rather than display:none so the button keeps its own width
11551163
and nothing on the row shifts while the work runs. Opacity stays up - a busy
@@ -2247,7 +2255,7 @@ <h3>Notifications</h3>
22472255
<button class="btn" id="saveSettings">Save</button>
22482256
<button class="btn ghost" id="discardSettings" type="button">Discard</button>
22492257
<span class="muted" id="settingsMsg"></span>
2250-
<button class="btn ghost" id="resetSettings" type="button" style="margin-left:auto">Reset to defaults</button>
2258+
<button class="btn ghost" id="resetSettings" type="button">Reset to defaults</button>
22512259
<button class="btn ghost" id="resetLayout" type="button">Reset tiles</button>
22522260
</div>
22532261
</div>
@@ -3442,6 +3450,7 @@ <h2><span class="drag-handle" title="Drag to reorder" aria-hidden="true"><svg vi
34423450
}
34433451
function openDrawer(){
34443452
dismissCoach(); // opening Settings marks the first-run coachmark seen
3453+
$('saveSettings').classList.remove('err'); // a discarded failure must not colour a fresh open
34453454
// Baseline the live theme preview + unsaved-changes snapshot on every real
34463455
// open (gear or error-bubble link) - but not if the drawer is already open,
34473456
// so a re-entrant open can't erase in-progress edits from the dirty check.
@@ -7790,11 +7799,17 @@ <h2><span class="drag-handle" title="Drag to reorder" aria-hidden="true"><svg vi
77907799
return b;
77917800
}
77927801
let saveInFlight=false;
7802+
// Marks the Save button as "this did not save". Text is optional: postAccess
7803+
// has already put its own error on screen, and only the button is missing.
7804+
function saveFailed(text){
7805+
if(text !== undefined) $('settingsMsg').textContent = text;
7806+
$('saveSettings').classList.add('err');
7807+
}
77937808
$('saveSettings').addEventListener('click', async () => {
77947809
// Ignore re-entrant clicks while a save is running: the access POST clears the password
77957810
// field mid-flight, so a second overlapping save could submit a blank password.
77967811
if(saveInFlight) return;
7797-
saveInFlight=true; $('saveSettings').disabled=true;
7812+
saveInFlight=true; $('saveSettings').disabled=true; $('saveSettings').classList.remove('err');
77987813
try {
77997814
// If the city/ID box still has text, search it first so settingsBody() captures
78007815
// the resulting scope - covers "typed but never clicked Search" and "changed the
@@ -7804,14 +7819,14 @@ <h2><span class="drag-handle" title="Drag to reorder" aria-hidden="true"><svg vi
78047819
// Don't save the old scope under a failed search - but an emptied box means the
78057820
// user gave up on that query (Auto location's programmatic clear included), so it
78067821
// no longer blocks. The failed text is kept in the box, so "fix" is followable.
7807-
if(serverSearchFailed && $('serverCity').value.trim()){ $('settingsMsg').textContent='Could not find that server/city - fix the search or clear the box before saving.'; return; }
7822+
if(serverSearchFailed && $('serverCity').value.trim()){ saveFailed('Could not find that server/city - fix the search or clear the box before saving.'); return; }
78087823
// Same rule for a bad iperf3 address: block rather than save around it, otherwise the
78097824
// save reports success while that field silently kept its old value.
78107825
const addrErr=iperfAddrError();
78117826
if(addrErr){
78127827
iperfAddrBad(addrErr);
78137828
activateTab('server'); $('setIperfAddr').focus();
7814-
$('settingsMsg').textContent='Fix the iperf3 server address before saving.';
7829+
saveFailed('Fix the iperf3 server address before saving.');
78157830
return;
78167831
}
78177832
// A renamed server whose stored password was never re-entered: the save would
@@ -7826,27 +7841,27 @@ <h2><span class="drag-handle" title="Drag to reorder" aria-hidden="true"><svg vi
78267841
{ const orphan=iperfPwOrphan();
78277842
if(orphan){
78287843
activateTab('server');
7829-
$('settingsMsg').textContent='Re-enter '+(orphan.label||orphan.orig_addr)+"'s password (its address changed), or change the address back, before saving.";
7844+
saveFailed('Re-enter '+(orphan.label||orphan.orig_addr)+"'s password (its address changed), or change the address back, before saving.");
78307845
return;
78317846
} }
78327847
const body = settingsBody(); // captures the open iperf3 editor (incl. a just-typed address) too
78337848
// iperf3 with no valid server set would fail every run ("no iperf3 server set") - block
78347849
// the save and point the user at the fix rather than persist a broken engine.
78357850
if(body.speed_engine==='iperf3' && !body.iperf_server){
7836-
$('settingsMsg').textContent='The iperf3 engine needs a server - add one with a host:port and pick it as the test target before saving.';
7851+
saveFailed('The iperf3 engine needs a server - add one with a host:port and pick it as the test target before saving.');
78377852
return;
78387853
}
78397854
// An enabled schedule whose windows have no active days never runs - block it
78407855
// rather than silently disabling probing/speedtests behind an "on" toggle.
78417856
for(const f of ['lat','speed']){ if($(SCHED_FEATS[f].en).checked && body['sched_'+f+'_windows'].every(w=>!w.days.includes('1'))){
7842-
$('settingsMsg').textContent='The '+(f==='lat'?'latency':'speedtest')+' schedule has no active days - pick a day or turn it off.'; return; } }
7857+
saveFailed('The '+(f==='lat'?'latency':'speedtest')+' schedule has no active days - pick a day or turn it off.'); return; } }
78437858
const msg=$('settingsMsg'); msg.textContent='Saving…';
78447859
try {
78457860
// Apply access changes first - setting a password re-issues the session
78467861
// cookie, so the settings POST that follows stays authenticated. Bail if it
78477862
// fails: postAccess has already shown the error, and we must not clear the
78487863
// typed password, POST the settings, or close the drawer as if it saved.
7849-
if(!await postAccess(accessBody())) return;
7864+
if(!await postAccess(accessBody())){ saveFailed(); return; }
78507865
$('authPass').value=''; $('authCurPass').value=''; // access (incl. any password) applied - safe to clear
78517866
const r=await fetch('api/settings',{method:'POST',headers:{'Content-Type':'application/json'},body:JSON.stringify(body)});
78527867
if(r.ok){ applySettings(await r.json());
@@ -7871,8 +7886,8 @@ <h2><span class="drag-handle" title="Drag to reorder" aria-hidden="true"><svg vi
78717886
if(lb) flashStatus('Saved - but '+(lb.label||lb.addr)+' points at localhost, which inside this bridged container is the container itself, not the machine running your iperf3 server. Use the host\'s LAN IP or host.docker.internal.'); } }
78727887
// Access was POSTed first (and applied); if the settings POST fails, say so -
78737888
// so Discard isn't mistaken for "nothing was saved".
7874-
else msg.textContent=(await r.text())+' (access/security settings, if changed, were already applied)';
7875-
} catch(e){ msg.textContent='failed'; }
7889+
else saveFailed((await r.text())+' (access/security settings, if changed, were already applied)');
7890+
} catch(e){ saveFailed('failed'); }
78767891
} finally { saveInFlight=false; $('saveSettings').disabled=false; }
78777892
});
78787893

internal/web/ui/ui.test.mjs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4948,3 +4948,36 @@ test('the heartbeat tip says Test is a real check-in', () => {
49484948
assert.match(tip, /\n Blank = off\./, 'both of them');
49494949
assert.doesNotMatch(tip, /simulat/, 'nothing here may read as a rehearsal');
49504950
});
4951+
4952+
test('a long settings message wraps inside its own column instead of moving the buttons', () => {
4953+
// Sized to its own text the message overflowed the action row, and the iperf3
4954+
// no-server warning pushed Reset tiles onto a line of its own.
4955+
const rule = html.match(/\n\s*#settingsMsg\s*\{[^}]*\}/);
4956+
assert.ok(rule, 'the message needs its own rule, or it sizes to its text and overflows the row');
4957+
assert.match(rule[0], /flex:\s*1 1 0/, 'it has to take the leftover space rather than demand its own');
4958+
assert.match(rule[0], /min-width:\s*0/,
4959+
'without this a long unbroken message refuses to shrink below its content width');
4960+
const foot = html.match(/id="resetSettings"[^>]*>/)[0];
4961+
assert.doesNotMatch(foot, /margin-left:auto/,
4962+
'the message column does the spacing now; two mechanisms for it drift apart');
4963+
});
4964+
4965+
test('every save that does not save turns the button red', () => {
4966+
const rule = html.match(/\n\s*\.btn\.err\s*\{[^}]*\}/);
4967+
assert.ok(rule, 'the failed-save state needs its own rule');
4968+
assert.match(rule[0], /var\(--down\)/,
4969+
'it has to use the palette down colour, so every theme gets its own red');
4970+
4971+
const at = html.indexOf("$('saveSettings').addEventListener('click'");
4972+
const end = html.indexOf('saveInFlight=false', at);
4973+
const handler = html.slice(at, end);
4974+
assert.ok(at > 0 && end > at, 'could not find the save handler');
4975+
// Every bail-out routes through saveFailed. A path that writes the message
4976+
// directly would explain itself and still leave the button looking normal.
4977+
assert.doesNotMatch(handler, /\$\('settingsMsg'\)\.textContent\s*=/,
4978+
'a blocked save wrote the message without marking the button');
4979+
assert.match(handler, /classList\.remove\('err'\)/,
4980+
'each attempt has to start clean, or a retry that works still looks failed');
4981+
assert.match(html.slice(html.indexOf('function openDrawer(')), /classList\.remove\('err'\)/,
4982+
'a failure that was discarded must not colour a freshly opened drawer');
4983+
});

0 commit comments

Comments
 (0)