feat(settings): add save feedback for preferences changes#2261
feat(settings): add save feedback for preferences changes#2261lspassos1 wants to merge 3 commits intokoala73:mainfrom
Conversation
|
@lspassos1 is attempting to deploy a commit to the Elie Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR introduces a shared Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant UnifiedSettings
participant renderPreferences
participant handlePreferenceChange
participant handleSettingsImport
participant showToast (global)
participant showToast (inline)
User->>UnifiedSettings: opens Settings → Preferences tab
UnifiedSettings->>renderPreferences: renderPreferences({ onSettingSaved: () => showToast("Saved") })
renderPreferences-->>UnifiedSettings: { html, attach() }
User->>renderPreferences: changes a preference (e.g. theme)
renderPreferences->>handleSettingsImport: handleSettingsImport(target, container)
handleSettingsImport-->>renderPreferences: false (not import input)
renderPreferences->>handlePreferenceChange: handlePreferenceChange(target, container, host)
handlePreferenceChange-->>renderPreferences: true (preference persisted)
renderPreferences->>showToast (global): host.onSettingSaved?.() → showToast("Saved")
showToast (global)-->>User: body-level toast for 3 000 ms
User->>renderPreferences: imports settings file
renderPreferences->>handleSettingsImport: handleSettingsImport(target, container)
handleSettingsImport->>showToast (inline): showToast(container, "Imported N keys", true)
showToast (inline)-->>User: inline #usDataMgmtToast message
handleSettingsImport-->>renderPreferences: true (import handled — early return)
Note over renderPreferences: onSettingSaved NOT called
User->>UnifiedSettings: clicks Save on Panels tab
UnifiedSettings->>UnifiedSettings: savePanelChanges()
UnifiedSettings-->>User: inline #usPanelsStatus "Saved" (no global toast)
Reviews (1): Last reviewed commit: "feat(settings): add save feedback for pr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
|
|
||
| it('removes duplicate global toast implementations from event handlers and country intel', () => { | ||
| assert.match(handlersSrc, /import \{ showToast \} from '@\/utils\/toast';/); |
There was a problem hiding this comment.
The pattern /private savePanelChanges\(\): void \{([\s\S]*?)\n {2}\}/ assumes the closing } of savePanelChanges is indented with exactly 2 spaces. Any formatting change (e.g., reformatter runs, indentation style shift) will silently break the assertion — saveMatch will be null, the assert.ok will fail with a confusing message, and the real assertion about showToast will never be reached.
Consider anchoring on the method name itself rather than its closing brace, or reading the entire file and checking the region between known method boundaries:
// alternative: just assert the method body does not contain showToast anywhere nearby
const methodStart = settingsSrc.indexOf('private savePanelChanges()');
const methodEnd = settingsSrc.indexOf('\n private ', methodStart + 1);
const methodBody = settingsSrc.slice(methodStart, methodEnd === -1 ? undefined : methodEnd);
assert.ok(methodBody.length > 0, 'savePanelChanges() not found');
assert.doesNotMatch(methodBody, /showToast\(/);| toast.classList.remove('visible'); | ||
| setTimeout(() => toast.remove(), 300); | ||
| }, 3000); |
There was a problem hiding this comment.
UnifiedSettings toasts
The old local showToast in UnifiedSettings.ts used a 4 000 ms dismiss timeout; event-handlers.ts and country-intel.ts used 3 000 ms. The new shared helper standardises on 3 000 ms, which silently shortens the display time for two toasts in UnifiedSettings (the free-panel-limit toast fired from toggleDraftPanel, and the new "Saved" toast). This may be intentional, but it's worth confirming — users who currently see the panel-limit warning for 4 s will now only see it for 3 s.
If 3 000 ms is the desired standard, a brief comment here would make the intention explicit:
}, 3000); // 3 s display — standardised across all global toasts
koala73
added
High Value
2 participants
Summary
Reapplies the settings save feedback work from #1269 on top of current
main, without carrying the stale branch drift that is now breaking that PR.Root cause
The app already had inline
Savedfeedback for the Panels tab, but Preferences changes (theme, map provider, language, AI toggles, etc.) still had no equivalent confirmation. The older PR branch also diverged badly frommainand no longer typechecked cleanly.Changes
src/utils/toast.tswithrole="status"renderPreferences()to acceptonSettingSavedand fire it once after persisted preference changesSavedtoast only for Preferences changesSavedstatus, so there is no duplicate feedback thereUnifiedSettings,event-handlers, andcountry-intelValidation
npx tsx --test tests/settings-save-feedback.test.mjsnpm run typechecknpx biome lint src/utils/toast.ts src/components/UnifiedSettings.ts src/services/preferences-content.ts src/app/event-handlers.ts src/app/country-intel.ts tests/settings-save-feedback.test.mjsRisk
Low risk. The change is limited to settings-related feedback paths and a shared toast helper, with the Panels tab intentionally left on its existing inline status behavior.
Ref #1247
Supersedes #1269