Fix file picker opening behind the main window (+ run it off the UI thread)

[ImmortalDragonm]

Problem

After importing an .entlayout, the whole top menu (and Export, and other buttons) stopped responding to clicks — the app looked alive but no menu would open. Originally reported as "Export does nothing after import"; it's actually every menu item.

Root cause (confirmed by instrumented repro)

import_entlayout_dialog / export_entlayout_dialog (and the entsettings + layout-image variants) call rfd's blocking FileDialog::pick_file()/save_file() directly inside the egui update() loop. On Linux rfd uses the xdg-desktop-portal backend (ashpd/D-Bus) and drives that request to completion on the calling thread. So:

1. The UI thread blocks for the whole duration of the dialog (app frozen — a driven Xvfb repro showed the render loop producing zero frames while the picker was in flight, stuck on the "Applying keyboard layout…" spinner).
2. Worse, after the blocking portal round-trip the egui/winit input state is left corrupted, so clicks stop registering afterwards — the dead menus.

Connection/layout state was proven fine throughout (layout stays Some, no disconnect), which is why the earlier connection-focused guesses didn't help.

Fix

Move every native picker onto a worker thread and deliver the chosen path back through an mpsc channel, polled each frame:

• New FileDialogAction + spawn_file_dialog / poll_file_dialog in src/ui/file_dialog.rs. One dialog at a time; the UI thread keeps rendering while it's open and never blocks on the portal.
• .entlayout import/export, .entsettings import/export, and layout-image export now spawn the dialog and do their read/write in a follow-up handler when the path arrives. Exports re-snapshot at write time so a long-open dialog can't write stale data.

Verification

Under Xvfb, with the dialog worker blocked on the portal, the UI still produced 200 render frames over ~6s (vs 0 before the change). cargo build, cargo test (133 passed), check_i18n.py all pass.

🤖 Generated with Claude Code

[ImmortalDragonm]

Update: the actual user-visible bug was that the native picker opened behind the main window (had to click twice to bring it forward), which made the app look frozen/dead after Import/Export. Added set_parent (latest commit) so the picker is parented to the main window and opens in front. The off-UI-thread change still stands so the window doesn't freeze while the picker is up.

[IgorArkhipov]

Could we keep this export tied to the device that opened the picker?

Since device polling continues while the picker is open, keyboard A can be replaced by B before this snapshot runs.
That would save B’s layout under A’s filename.

Capturing the request-time snapshot or rejecting a connection-generation mismatch would avoid that. Same concern applies to layout-image export.

[IgorArkhipov]

Could we add focused tests for this new state machine?

Useful cases: Empty, completion with and without a path, Disconnected, second-dialog rejection, every action route, and post-close input recovery.

The Xvfb check confirms rendering continues, but does not cover these completion paths.

[IgorArkhipov]

Could this branch run the same dropdown, pointer, text-input, and repaint cleanup as normal completion?

If the picker worker panics, the sender disconnects and we currently clear only pending_file_dialog, potentially leaving input state wedged without a visible error.

[IgorArkhipov]

Could we bind device-scoped dialog requests to a connection generation or device identity?

While the picker is open, keyboard A can disconnect and B can auto-connect.

This completion then starts the import against current hid_device, which could program B with state intended for A.

Rejecting stale completion with a retry message would make this safe.

[IgorArkhipov]

Small cleanup: _ctx is unused here, and the result dispatcher appears to pass egui::Context only for this parameter.

Could we remove that plumbing, along with the unused context on import_entsettings_dialog, so poll_file_dialog remains the sole owner of input recovery?

[ImmortalDragonm]

Thanks for the review! Addressed:

• Device race: added a connection generation bumped on every connect/disconnect. A file dialog captures it when opened; a device-scoped result (entlayout import/export, layout-image export) is discarded with a retry message if the device changed while the picker was up. App-settings import/export stay device-independent. Applies to both the export-filename and the import-target concerns.
• Disconnected branch: a vanished/panicked worker now runs the same dropdown / pointer / text-input / repaint recovery as a normal completion, so a lost dialog can't leave input wedged.
• Cleanup: dropped the unused egui::Context plumbing (write_layout_image_export, import_entsettings_dialog) so poll_file_dialog is the sole owner of input recovery.
• Added unit tests for the device-scoped classification and the staleness decision.

For the full state-machine cases (Empty / second-dialog rejection / every route) I covered the pure decision logic; the rest lives behind a constructed EntropyApp. Happy to add a lightweight harness if you'd like those exercised end-to-end.

[kissetfall]

There is still a device-swap race for ImportEntlayout.

The dialog result is checked against connection_generation in poll_file_dialog, but ImportEntlayout only stores the path in pending_entlayout_import_path. The real firmware write happens later in handle_pending_imports / import_entlayout_from_path, after a delay. A device can still disconnect/reconnect in that gap, so an .entlayout chosen for keyboard A can be applied to keyboard B.

Please carry the opened connection generation into the deferred pending import, re-check it immediately before import_entlayout_from_path, and drop the pending import with the retry message if it changed. A focused test for this deferred-import stale-generation path would close the loop.

[ImmortalDragonm]

Good catch on the deferred import — fixed. The pending .entlayout import now carries the connection generation captured when the file was chosen, and handle_pending_imports re-checks it immediately before import_entlayout_from_path, dropping the import with the retry message if the device changed in that gap. Factored the rule into device_generation_stale (shared by the dialog-completion and deferred-write checks) with a focused test. Also ran rustfmt on the changed files. cargo test 137/137.

[IgorArkhipov]

Thanks for adding this check.

Could we make it exercise production wiring?
This test calls only device_generation_stale, while handle_pending_imports performs a separate comparison, so it stays green if the deferred guard is removed or miswired.

Routing production through one tested helper or using a small app fixture would cover the regression; the same fixture can drive Empty, completion, Disconnected, routing, and recovery.

[ImmortalDragonm]

handle_pending_imports now calls the same device_generation_stale helper the unit test covers (instead of an inline comparison), so removing/miswiring the deferred guard would fail the shared decision. A full app-fixture path (Empty/completion/Disconnected/routing/recovery) is a larger harness; can add it as a follow-up if you'd like end-to-end coverage.

[kissetfall]

The centralized file-dialog owner is the right architectural direction, but this needs another pass before merge.

1. Rebase onto current main and resolve the conflicts in Cargo.toml and src/ui/app_lifecycle.rs.
2. Run rustfmt on the changed files. The current head fails the formatting check in src/app.rs and src/entsettings.rs.
3. Localize the user-visible “Device changed while the file dialog was open” messages in both file_dialog.rs and the deferred-import path instead of hardcoding English.
4. Do not silently swallow a disconnected dialog worker. TryRecvError::Disconnected currently clears the pending state without an actionable status/error; report the failure to the user and log it.
5. Add state-machine tests for the real dialog lifecycle. The current tests cover helper predicates, but not result dispatch. Please cover success, cancel, stale device generation, deferred .entlayout re-check, worker disconnect, and the one-dialog-at-a-time invariant/input recovery.

Ready for re-review when the rebased branch merges cleanly, formatting and i18n checks pass, and the lifecycle regression tests cover those paths.

[ImmortalDragonm]

Addressed the review and rebased onto current main:

Localized the stale-device message. The "device changed while the file dialog was open" text is now in the i18n catalog (status_messages.file_dialog_device_changed, en + ru) and used by both the dialog-completion path and the deferred .entlayout write, instead of hardcoded English.

A lost worker is no longer swallowed. TryRecvError::Disconnected (dialog thread panicked/vanished before sending a result) now logs an error and shows a localized file_dialog_worker_lost status, in addition to the existing state-clear + input recovery — so a lost dialog surfaces instead of silently doing nothing.

Testable lifecycle. Extracted the poll decision into classify_file_dialog_poll → FileDialogPoll and the single-slot guard into can_start_file_dialog, then drove the method through them. New tests cover the full state machine: pending (keeps waiting), success dispatch, cancel, stale device (and that app-settings actions ignore device changes), worker disconnect, the one-dialog-at-a-time invariant, and that every terminal outcome frees the slot.

Formatting. Applied rustfmt to the app.rs module ordering that was flagged.

cargo test 263 passing, cargo fmt --check clean on the touched files, check_i18n.py clean.

[kissetfall]

Повторное ревью d078872.

Предыдущие замечания по самой file-dialog lifecycle закрыты: проверка generation переиспользуется перед deferred import, stale/worker-lost сообщения локализованы, разрыв worker-канала больше не проглатывается, а state machine покрывает pending/success/cancel/stale/disconnect и single-flight.

Остался блокирующий integration-пункт:

• Ветка больше не совместима с текущим main (e972a45): GitHub показывает DIRTY, подтверждён content-conflict в src/ui/layout_image_export.rs с уже смерженным #78. Текущий head знает только PNG/SVG, поэтому при разрешении конфликта важно не потерять PDF: сохранить его filter/extension/render/write path и провести его через новый асинхронный parented file-dialog lifecycle. После rebase нужен regression test/проверка, что PNG, SVG и PDF все открывают picker через worker и корректно экспортируются.

На текущем head локально прошли 263/263 теста, release-сборка, scoped rustfmt, i18n и git diff --check. Hosted checks для head отсутствуют. После rebase на актуальный main и зелёного CI можно будет мержить.

[ImmortalDragonm]

Rebased onto current main (36defd4), resolving the layout_image_export.rs conflict with the merged PDF export (#78).

PDF was preserved and folded into the async lifecycle. The merged #78 opened its picker synchronously on the UI thread; PDF now goes through the same off-thread parented file-dialog path as PNG/SVG. The picker (spawn) and the writer read a single shared layout_image_export_descriptor, so all three formats dispatch spawn_file_dialog(ExportLayoutImage) and none can diverge on extension/filter. Render/write still routes Pdf → render_layout_pdf.

Regression tests assert every format resolves to a distinct extension + picker filter, and that the picker and writer agree on the extension for PNG, SVG, and PDF — the exact drift the merge could have introduced.

Also dropped the duplicate raw-window-handle entries the earlier rebase had left under the windows/macos targets; the not(target_arch = "wasm32") target already covers every desktop OS.

cargo test 321 passing, release build, scoped cargo fmt --check, check_i18n.py, and git diff --check all clean. Branch now merges cleanly against main.

[kissetfall]

Re-review complete against current main (7be7b00). The previous blockers are resolved: dialog results are device-generation scoped, worker disconnects surface localized errors, state transitions have regression coverage, and PNG/SVG/PDF all use the shared asynchronous parented-dialog path. Verified a clean merge with 322/322 tests, i18n, diff-check, scoped rustfmt, and a release build; CI is 4/4 green. Approved.