-
Notifications
You must be signed in to change notification settings - Fork 17
fix: confirm desktop close behavior before hiding to tray #118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
zouyonghe
wants to merge
18
commits into
AstrBotDevs:main
Choose a base branch
from
zouyonghe:codex/close-to-tray-confirmation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
fd71afb
chore(version): sync desktop version to v4.23.0-beta.1
github-actions[bot] 4b9e443
fix: add backend startup heartbeat liveness probe (#114)
zouyonghe f3db42a
Merge remote-tracking branch 'upstream/main'
zouyonghe 98c570f
Merge remote-tracking branch 'upstream/main'
zouyonghe 7da9e76
fix: confirm desktop close behavior before hiding to tray
zouyonghe dee9ab5
refactor: simplify close prompt action persistence
zouyonghe b74fcdf
refactor: inline close prompt action handling
zouyonghe b8873db
refactor: share desktop close exit handling
zouyonghe 1e24289
fix: clarify close prompt cleanup log
zouyonghe dfc0ce2
refactor: sync close confirm action values
zouyonghe 8adb9ea
refactor: inject close prompt logging callbacks
zouyonghe 99a4845
refactor: streamline close behavior persistence
zouyonghe ac4e7d1
refactor: simplify close behavior io helpers
zouyonghe ee14bae
fix: surface close preference save failures
zouyonghe 0392943
test: fix clippy needless borrow in close behavior
zouyonghe 45230cd
fix: avoid false exit errors in close prompt
zouyonghe 7acc59c
fix: keep main window visible when prompt fails
zouyonghe e5d7b14
test: remove unused close prompt fallback helpers
zouyonghe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import path from 'node:path'; | ||
| import { readFile } from 'node:fs/promises'; | ||
| import { test } from 'node:test'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| const scriptDir = path.dirname(fileURLToPath(import.meta.url)); | ||
| const projectRoot = path.resolve(scriptDir, '..', '..'); | ||
| const htmlPath = path.join(projectRoot, 'ui', 'close-confirm.html'); | ||
|
|
||
| const readHtml = async () => readFile(htmlPath, 'utf8'); | ||
|
|
||
| test('close confirm dialog uses locale copy as the single source of truth for button labels', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.match(html, /trayButton\.textContent = copy\.tray;/); | ||
| assert.match(html, /exitButton\.textContent = copy\.exit;/); | ||
| }); | ||
|
|
||
| test('close confirm dialog avoids exposing raw invoke errors to users', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.doesNotMatch(html, /invokeError\.message/); | ||
| assert.match(html, /error\.textContent = copy\.submitError;/); | ||
| }); | ||
|
|
||
| test('close confirm dialog routes Tauri command calls through a local invoke wrapper', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.match(html, /const invokeTauri =/); | ||
| assert.doesNotMatch(html, /window\.__TAURI_INTERNALS__\?\.invoke/); | ||
| assert.match(html, /await invokeTauri\(/); | ||
| }); | ||
|
|
||
| test('close confirm dialog reads close action values from query params instead of hard-coded literals', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.match(html, /const trayAction = params\.get\("trayAction"\);/); | ||
| assert.match(html, /const exitAction = params\.get\("exitAction"\);/); | ||
| assert.doesNotMatch(html, /submit\("tray"\)/); | ||
| assert.doesNotMatch(html, /submit\("exit"\)/); | ||
| }); | ||
|
|
||
| test('close confirm dialog only schedules frontend close fallback for tray actions', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.match(html, /if \(action === trayAction\) \{/); | ||
| assert.match(html, /recoveryTimer = window\.setTimeout\(/); | ||
| assert.match(html, /window\.close\(\);/); | ||
| }); | ||
|
|
||
| test('close confirm dialog suppresses invoke teardown errors for exit actions', async () => { | ||
| const html = await readHtml(); | ||
|
|
||
| assert.match(html, /catch \(_invokeError\) \{/); | ||
| assert.match(html, /if \(action === exitAction\) \{/); | ||
| assert.match(html, /return;/); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider inlining the close-prompt cleanup and parsing logic into the command to avoid extra helper layers that add indirection without clear benefit.
You can reduce indirection here without losing any functionality.
1. Remove
finish_tray_close_prompt_cleanupindirectionThe generic helper is only used once and always with
append_desktop_log. You can inline the logic and drop the generic function and its tests:Then you can delete
finish_tray_close_prompt_cleanupand its tests.2. Avoid wrapping
parse_close_actionwith another parserInstead of
parse_close_prompt_action, you can map theOptionfromclose_behavior::parse_close_actiondirectly at the call site, keeping only one parsing API:If you still want a testable parsing function, you can move an ergonomic
Result-returning parser intoclose_behavioritself:And in the command:
This removes the extra abstraction layer in the bridge module and keeps the flow in
desktop_bridge_submit_close_promptmore direct.