Skip to content

feat: add unsaved changes confirmation to settings page (Closes #1086) - #1154

Merged
K1NGD4VID merged 9 commits into
LabsCrypt:mainfrom
sojetunde8:fix/issue-1086-unsaved-changes-settings
Sep 1, 2026
Merged

feat: add unsaved changes confirmation to settings page (Closes #1086)#1154
K1NGD4VID merged 9 commits into
LabsCrypt:mainfrom
sojetunde8:fix/issue-1086-unsaved-changes-settings

Conversation

@sojetunde8

Copy link
Copy Markdown
Contributor

Closes #1086

Summary

  • Track dirty state by comparing initial vs current values for all editable settings fields
  • Add native beforeunload handler to warn on tab close, browser refresh, and back/forward navigation
  • Add window.confirm prompts for in-app navigation via the "Connect Wallet" button and "Disconnect Wallet" button when dirty
  • Replace the "Connect Wallet" <Link> with a <button> to intercept navigation with confirmation

Testing

  • Added 7 unit tests covering: clean start, dirty detection for each field (theme, currency, format, decimals), no-confirm when unchanged, and disconnect flow without changes

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good instinct on #1086 and the test coverage is appreciated, but two design problems block this:

  1. settings on main persist immediately (toggleTheme writes localStorage on click), yet isDirty stays true forever after any change, so users get warned about "unsaved" changes that are already saved. either introduce a real draft-plus-save model or reset initialValuesRef after each apply.
  2. the guard only wraps the two in-page buttons. all navbar links and any other internal Link navigation bypass it entirely, so the stated goal is not met. cover app-internal navigation or scope the issue down explicitly.
  3. small ones: the Link to button swap loses link semantics, and there is a stray blank-line diff where handleDisconnect was touched.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

Comment thread frontend/src/hooks/useUnsavedChangesGuard.ts Fixed
@sojetunde8

Copy link
Copy Markdown
Contributor Author

good instinct on #1086 and the test coverage is appreciated, but two design problems block this:

  1. settings on main persist immediately (toggleTheme writes localStorage on click), yet isDirty stays true forever after any change, so users get warned about "unsaved" changes that are already saved. either introduce a real draft-plus-save model or reset initialValuesRef after each apply.
  2. the guard only wraps the two in-page buttons. all navbar links and any other internal Link navigation bypass it entirely, so the stated goal is not met. cover app-internal navigation or scope the issue down explicitly.
  3. small ones: the Link to button swap loses link semantics, and there is a stray blank-line diff where handleDisconnect was touched.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

Rsolved, @ogazboiz

sojetunde8 and others added 5 commits September 1, 2026 08:09
…anges feature

- Remove duplicate `copied` state declaration in SettingsContent
- Add missing `handleSave` function that persists draft settings and resets dirty state
- Add missing `useUnsavedChangesGuard` import
- Mock `DisconnectConfirmModal` in tests so disconnect flow assertions work

Closes LabsCrypt#1086
…in stream details

- Correct eslint-disable-next-line rule name from react-compiler/react-compiler
  to react-hooks/preserve-manual-memoization to suppress React Compiler
  memoization warning on liveClaimable useMemo
- Fix stream-details-content test using getAllByText instead of getByText
  since the claimable amount appears twice (visible + screen-reader text)
TypeScript build fails with TS6133 when strictUnusedLocals is enabled.
The React import was not needed since the component uses JSX transform.
@K1NGD4VID
K1NGD4VID merged commit 2f8a1b2 into LabsCrypt:main Sep 1, 2026
12 checks passed
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.

[Frontend] settings-content.tsx does not confirm before discarding unsaved changes on navigation

5 participants