fix(windows): make QA panel close button clickable#240
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2ba637ee5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Err(e) = window.show() { | ||
| log::warn!("[qa] show failed: {e}"); |
There was a problem hiding this comment.
Replacing the Windows path with window.show() drops the only explicit SWP_NOACTIVATE/SW_SHOWNOACTIVATE handling, so opening the QA panel can reactivate OpenLess. That breaks the contract documented in show_qa_window (must not steal foreground focus) and can regress selection capture: begin_qa_session relies on the previously focused app remaining frontmost so AX/Cmd+C fallback reads the user’s selected text, not the QA webview. In practice, after opening QA on Windows, subsequent recording sessions may capture empty/wrong selection because focus has shifted.
Useful? React with 👍 / 👎.
PR Reviewer Guide 🔍(Review updated until commit 337eaeb)Here are some key observations to aid the review process:
|
…ction Issue Open-Less#239 required the close button to be clickable, but a plain window.show() regression can steal focus and break selection capture assumptions in QA flow. Keep an explicit no-activate show on Windows while simplifying the native call path to only SW_SHOWNOACTIVATE. Constraint: QA panel must not steal foreground focus when opening Rejected: Keep plain window.show() | violates non-activation contract and risks wrong selection source Confidence: medium Scope-risk: narrow Directive: If Windows QA show logic changes again, verify both close-button clickability and front-app selection capture Tested: cargo check -q (openless-all/app/src-tauri) Not-tested: Manual Windows interaction on physical device
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Persistent review updated to latest commit 337eaeb |
包含自 1.2.13 以来的修复与新功能： - feat(autostart): 跨端开机自启 (#194) - feat(asr): SiliconFlow / GLM-ASR / Groq / OpenAI Whisper preset (#212) - fix(qa): Windows / Linux 划词追问浮窗 Esc + 拖拽 + 文案 (#205 / #206) - fix(settings): preset 切换 race + per-provider 凭据隔离 (#219 / #220) - fix(overview): 概览整屏适配 + 嵌套 scroller 细滚动条 (#243 / #248) - 一系列 Windows IME (TSF) 模块 (#233 / #240 等)
1 participant
User description
Summary
Validation
Closes #239
PR Type
Bug fix
Description
SetWindowPosfrom show pathDiagram Walkthrough
flowchart LR A["Windows QA window show logic"] -- "simplify" --> B["Use ShowWindow(SW_SHOWNOACTIVATE) only"] B -- "improves" --> C["Close button receives clicks"]File Walkthrough
lib.rs
Simplify Windows QA window show behavioropenless-all/app/src-tauri/src/lib.rs
show_qa_window_no_activateon Windows to call onlyShowWindow(SW_SHOWNOACTIVATE).SetWindowPos(..., HWND_TOPMOST, SWP_NOACTIVATE)step.
window.show()when native no-activatedisplay fails.