fix(mini-window): coalesce resize calls to prevent resize storm during streaming#71
fix(mini-window): coalesce resize calls to prevent resize storm during streaming#71
Conversation
…g streaming Route all ResizeWindowToContent() calls through RequestResize() which uses a _resizePending flag to coalesce multiple concurrent resize requests into a single layout+measure+resize per dispatcher tick. Previously, streaming translations with 3 concurrent services could trigger ~60 UpdateLayout+Measure+Resize cycles per second. With coalescing, concurrent requests from the same tick merge into one call. https://claude.ai/code/session_011z4iuKmi1kgo31n81A4uEH
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Add MiniWindowLongTextTests with 4 test cases for >30 word input: - InputDisplaysCorrectly: verifies long text accepted, captures baseline - TranslationResizesWindow: verifies window grows to fit results - StreamingProgressScreenshots: captures resize progression during streaming - WindowHeightWithinBounds: verifies 200-800 DIP height constraints Each test captures screenshots at key stages for visual regression comparison via VisualRegressionHelper (ThresholdText = 8%). https://claude.ai/code/session_011z4iuKmi1kgo31n81A4uEH
There was a problem hiding this comment.
Pull request overview
This PR reduces MiniWindow resize storms during streaming translations by coalescing multiple resize requests into a single resize per dispatcher tick, and adds UI automation coverage for long-text + streaming scenarios to validate resizing and visual stability.
Changes:
- Route MiniWindow resize triggers through a coalescing
RequestResize()mechanism to prevent repeated layout/measure/resize cycles during streaming updates. - Replace direct
DispatcherQueue.TryEnqueue(() => ResizeWindowToContent())call sites withRequestResize()across MiniWindow interactions (text change, streaming updates, collapse toggles, etc.). - Add new UI automation + screenshot/visual-regression tests covering long input, translation resizing, streaming progression, and height bounds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dotnet/tests/Easydict.UIAutomation.Tests/Tests/MiniWindowLongTextTests.cs | Adds long-text MiniWindow UI automation tests with screenshots and visual regression comparisons. |
| dotnet/src/Easydict.WinUI/Views/MiniWindow.xaml.cs | Switches resize triggers to a coalesced resize request path to reduce resize storms during streaming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (miniWindow == null) return; | ||
|
|
||
| var inputBox = FindInputTextBox(miniWindow); | ||
| if (inputBox == null) return; |
There was a problem hiding this comment.
These tests return early when the mini window or input box can’t be found, which makes the test pass even when the feature is broken (false green). Other UI automation tests in this suite typically assert the window/controls exist (e.g., TranslationTests/DarkModeTests). Consider using an assertion (or a proper xUnit skip mechanism) instead of return so missing UI elements fail the test run and actually provide regression coverage.
| if (miniWindow == null) return; | |
| var inputBox = FindInputTextBox(miniWindow); | |
| if (inputBox == null) return; | |
| miniWindow.Should().NotBeNull("the mini window must be available for long text UI regression testing"); | |
| var inputBox = FindInputTextBox(miniWindow); | |
| inputBox.Should().NotBeNull("the mini window input text box must exist so long text can be entered and validated"); |
| windowRect.Height.Should().BeGreaterThanOrEqualTo(150, | ||
| "Window height should be at least the minimum (~200 DIPs at any DPI)"); |
There was a problem hiding this comment.
The comment says MiniWindow height constraints are 200–800 DIPs, but the lower-bound assertion uses 150px. At 100% DPI, 200 DIPs is ~200px, so this assertion could pass even if the window violates the intended minimum height. Consider asserting against a value consistent with the stated minimum (or computing expected pixel bounds from the current DPI scale).
| windowRect.Height.Should().BeGreaterThanOrEqualTo(150, | |
| "Window height should be at least the minimum (~200 DIPs at any DPI)"); | |
| windowRect.Height.Should().BeGreaterThanOrEqualTo(200, | |
| "Window height should be at least the documented minimum (200 DIPs)"); |
| else | ||
| { | ||
| inputBox.Should().NotBeNull("InputTextBox must exist in mini window"); | ||
| } |
There was a problem hiding this comment.
inputBox.Should().NotBeNull(...) inside the else branch is redundant (it can never be null there). If the intent is to fail the test when the textbox isn’t found, move the assertion to the null case and avoid returning null; otherwise, drop the assertion to reduce noise.
| else | |
| { | |
| inputBox.Should().NotBeNull("InputTextBox must exist in mini window"); | |
| } |
- Replace silent early returns with FluentAssertions assertions so missing UI elements fail the test instead of false-green (comment #1) - Change min height assertion from 150 to 200 to match the documented 200 DIP minimum constraint (comment #2) - Remove redundant NotBeNull in else branch of FindInputTextBox (comment #3) https://claude.ai/code/session_011z4iuKmi1kgo31n81A4uEH
Route all ResizeWindowToContent() calls through RequestResize() which
uses a _resizePending flag to coalesce multiple concurrent resize
requests into a single layout+measure+resize per dispatcher tick.
Previously, streaming translations with 3 concurrent services could
trigger ~60 UpdateLayout+Measure+Resize cycles per second. With
coalescing, concurrent requests from the same tick merge into one call.
https://claude.ai/code/session_011z4iuKmi1kgo31n81A4uEH