[Quick Accent] Fix window width when descriptions are disabled - #49402
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hi John Surles (@0utsights). Thanks for your contribution. Please note that we do not accept untested PRs at this time. This is because of the number of AI-authored PRs we receive which often do not even compile. Please supply screenshots and/or a screen recording to show that your fix works as intended. Thanks. |
|
Thanks for the guidance. I've now built and tested the change locally. Validation:
Patched build result:
|
|
@microsoft-github-policy-service agree |
|
Thanks for the screenshot. I've built this myself now. Unfortunately, I can see that there is still not enough space to accommodate the required width, which results in the character list shifting when selection changes: Screen.Recording.2026-07-20.213455.mp4 |
Dave Rayment (daverayment)
left a comment
There was a problem hiding this comment.
Suggested changes to add the necessary 2 DIPs and document the update.
…rControl.xaml.cs Co-authored-by: Dave Rayment <dave.rayment@gmail.com>
…dow.xaml.cs Co-authored-by: Dave Rayment <dave.rayment@gmail.com>
…dow.xaml.cs Co-authored-by: Dave Rayment <dave.rayment@gmail.com>
|
Hey! Sorry, I just reviewed my version and your changes and realized I totally overlooked it. Thank you for pointing it out and fixing it! |
Dave Rayment (daverayment)
left a comment
There was a problem hiding this comment.
Thanks for integrating the suggested updates.
Just to wrap things up, could you update the PR description to account for the change in the calculation, please? Otherwise, this now looks fine.
Dave Rayment (daverayment)
left a comment
There was a problem hiding this comment.
Sorry, after some subsequent testing, it looks like there are still issues with the selection window not being wide enough on some displays when scaling has been applied.
For example, on my laptop display at 125% scaling, everything's fine, but on a 4K external monitor, the problem reoccurs:
QA4K175.mp4
|
John Surles (@0utsights) I think the final issue is because of fractional pixel values involved in the layout when the display is scaled. This is fine at some scales (where the fractional value is rounded down), but not in others (where it's rounded up, leading to the window being one pixel too narrow for the content). I'd suggest:
// Handles the fractional pixels that may occur with scaled displays from truncating the character list.
private const double LayoutRoundingDip = 1;In double contentWidthDip = (ViewModel.Characters.Count * ItemWidthDip) + Selector.HorizontalSurfaceOverheadDip + LayoutRoundingDip;What do you think? I've tested this and it works for my displays at 125% and 175%, but please corroborate on your own system. |
|
Hey! I was able to reproduce the issue at both 150% and 175% scaling. I then tested your suggested change, and it works great at both scales. I’ll apply the change and update the PR. Thanks for investigating this! |
Dave Rayment (daverayment)
left a comment
There was a problem hiding this comment.
Thanks for the update and for testing. Just minor comment nits to clear up now, I think.
…dow.xaml.cs Co-authored-by: Dave Rayment <dave.rayment@gmail.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
…easured width (#49633) ## Summary of the Pull Request Two defects in the WinUI 3 Quick Accent selector. They look unrelated but share a root: the overlay owns no layout of its own, so `MainWindow` sizes and shows it by hand — and both halves of that hand-rolled logic rest on an assumption that does not hold. * **#49489** — the bar appears blank, too wide and clipped on the right for a few frames, then snaps into place. A hidden WinUI 3 window renders nothing, so `ShowWindow(SW_SHOWNA)` puts the HWND on screen before the freshly rebuilt accent list has ever been laid out. * **#49488** — the window is sized narrower than its own content whenever a glyph is wider than the 48 DIP cell, so the list silently scrolls inside it and the trailing accents are pushed against the right edge. test result: https://github.com/user-attachments/assets/80d57aa8-5030-46c3-9ec8-6ed05ed00f03 ## PR Checklist - [x] Closes: #49489 - [x] Closes: #49488 - [x] **Communication:** bug fixes for two open, triaged issues in an existing module; no new feature surface - [x] **Tests:** added — 11 cases in `PowerAccent.Core.UnitTests` covering the new `Calculation.GetToolbarWidth`; the existing positioning/DPI suites are unaffected - [x] **Localization:** no new end-user-facing strings - [ ] **Dev docs:** N/A — the *Toolbar Sizing and Reveal* section was dropped from this PR; the reasoning lives in the code comments and in the commit messages instead - [x] **New binaries:** none — no new projects or outputs, so no `ESRPSigning_core.json`, `Product.wxs`, CI or release YML changes are needed - [ ] **Documentation updated:** N/A — internal rendering/layout fix with no user-facing behavior change beyond the bugs going away ## Detailed Description of the Pull Request / Additional comments ### #49489 — the blank, over-wide, clipped first frame The window is never actually repositioned or resized. Measuring the two frames in the issue shows the same HWND rect in both: identical left edge, and the bad frame's hard right cut sits exactly where the good frame's rounded corner plus its 24 DIP margin ends. What changes is the **content** — it is composed once from a stale layout, then re-laid-out. Two ordering problems produced that stale composition: 1. `TransientSurface` is `Collapsed` while hidden and is only flipped to `Visible` from the `Showing` event, which `TransparentWindow.RaiseShow` raises **after** `ShowWindow(SW_SHOWNA)`. The accent bar's subtree therefore provably has not been measured or arranged at the moment the HWND becomes visible. 2. The hide path called `ViewModel.Characters.Clear()` synchronously right after `Hide()` — but `Hide()` only *queues* the dismissal, so the still-visible window rendered an empty bar at the old width. That empty card is exactly what the next summon put back on screen. The fix mirrors what the WPF implementation did for the same symptom in #46593 (render off screen, then `SetWindowPos` into view), adapted to WinUI 3 where a hidden window does not render at all: * Show the bar with `Selector.Opacity = 0`, lay it out, and unveil it once `CompositionTarget.Rendering` confirms a couple of frames have elapsed. `Opacity = 0` still renders (unlike `Visibility.Collapsed`), which is exactly what is needed here. A 150 ms timeout backs it up — not because frames stop arriving (attaching a `Rendering` handler forces the UI thread to run every frame) but because the tick cadence carries no guarantee and can stop for a locked or fully occluded session; on that path the bar simply appears the way it used to, so it can never get stuck invisible. * Size the bar **twice** per summon: once before `Show`, and again after the first real layout pass. The first measurement runs while the surface is still `Collapsed` and, on the first summon of the process, before its template has ever been applied, so it can report less than the items need. The correction happens while the bar is still transparent, so it is never seen as a resize. * Leave the characters in the list on hide. The next summon clears and refills them anyway, and not clearing them removes the blank-bar frame at the source. * A generation counter drops a pending reveal when the summon is dismissed or superseded before its frame lands, and arming a new summon detaches the previous one's per-frame handler so it cannot unveil the new bar ahead of its own layout pass. ### #49488 — width derived from the item count instead of measured `MainWindow` computed the window width as `Characters.Count * 48`, while the XAML cell is `MinWidth="48"` — a *minimum*, not a fixed width. `ListViewItem` → `Grid MinWidth=48` with a `ContentPresenter Margin=12`, so a cell is `max(48, glyphWidth + 24)`: any glyph wider than 24 DIP (₹, ‰, ﷼, ៛, CJK fallbacks) grows its cell. With **All languages** selected, R and P each carry ~20 characters and the accumulated error is enough for the real content to overflow the window. The ListView's `ScrollViewer` (`HorizontalScrollMode="Enabled"`, `HorizontalScrollBarVisibility="Hidden"`) then absorbs the overflow invisibly, and `ScrollIntoView` starts scrolling a bar that should not scroll at all. The pre-migration WPF window used `SizeToContent="WidthAndHeight"` and only set `MaxWidth`, so the layout system measured the same item template and the window simply grew — which is why this never showed up before. `AppWindow` has no `SizeToContent` equivalent, and the migration replaced it with a constant model. Now: * `SelectorControl.MeasureContentWidthDip()` measures the list against an unbounded width and returns what the items actually need. Measuring explicitly, rather than reading a stale `DesiredSize`, addresses the concern recorded in the original comment: the bar is rebuilt on every summon while the window is still hidden, so no layout pass has run for the new items yet. * `Calculation.GetToolbarWidth()` — a pure function, hence the unit tests — floors that measurement at `itemCount * minItemWidth` (every cell is at least the minimum, so a list that could not be measured reports 0 and safely falls back to the old estimate instead of collapsing the bar), applies the description row's minimum width, and clamps to the display's usable width so long character sets still scroll on purpose. * The clamp's lower bound is `minItemWidth + chromeWidth` — one cell plus the space around it, the narrowest bar that can still draw a glyph — and its upper bound is `Math.Max(minItemWidth + chromeWidth, maxWidth)`, because a display narrower than that floor would otherwise invert the bounds and make `Math.Clamp` throw. `DescriptionMinWidthDip = 648` masked this bug whenever the Unicode description row was on and the character set short, which is likely why #49402 (description-row width) did not surface it. ## Validation Steps Performed * `PowerAccent.Core`, `PowerAccent.UI` and `PowerAccent.Core.UnitTests` build clean (Debug|x64). * `PowerAccent.Core.UnitTests`: 32/32 pass, including the 11 `GetToolbarWidth` cases — narrow glyphs hug the item count, wide glyphs win over the count estimate (the #49488 regression guard), the measurement winning by a single DIP, a partly realized list keeping the item-count floor, an unmeasured list falling back to the estimate, over-long content clamping to the display maximum, the description row widening a short bar but not a long one, the description minimum losing to a narrower display, and both ends of the clamp. The lower clamp bound was verified by mutation: rewriting it to `Math.Clamp(width, 0, ...)` fails only `GetToolbarWidth_EmptyList_FallsBackToOneCellPlusChrome`. --------- Co-authored-by: Yu Leng (from Dev Box) <yuleng@microsoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Summary of the Pull Request
Fixes Quick Accent clipping or horizontally shifting the last character when Unicode descriptions are disabled and the character list is short.
The WinUI window width was calculated as
item count × 48 DIPs, but the selector surface also has 24-DIP left and right margins and a 1-DIP border on each side. Those values reduced the usable list width. Fractional layout rounding at scaled display settings could then leave the viewport one physical pixel too narrow even after accounting for the nominal XAML dimensions.The sizing calculation now reads the surface's live horizontal margin and border thickness and includes them in the requested window width. It also adds a 1-DIP layout-rounding allowance so the character list is not truncated at fractional display scales.
PR Checklist
Detailed Description of the Pull Request / Additional comments
SelectorControl.xamlgives theTransientSurfaceaMargin=24,24,24,16, andDefaultTransientSurfaceStylesupplies a 1-DIP border on each side. For the four-character reproduction in #49346, the previous calculation requested a 192-DIP window (4 × 48). After the 48 DIPs of horizontal surface margin, only 144 DIPs remained for the list, which is exactly three character cells.SelectorControlnow exposes the computed left-plus-right surface margin and border thickness internally.MainWindow.SizeAndPosition()adds that live overhead to the character-driven width before applying the existing description minimum and monitor-width clamp. A further 1-DIP allowance covers fractional physical-pixel rounding at scaled display settings.With four characters, the calculation reserves the complete 192-DIP list width, the 50-DIP surface overhead, and the 1-DIP layout-rounding allowance. This leaves long-list scrolling, selected-character scrolling, description sizing, monitor clamping, DPI conversion, and window positioning unchanged.
Validation Steps Performed
git diff --checkpasses.PowerAccent.UIlocally with Visual Studio 2026 inDebug|x64; the build completed successfully with 0 warnings and 0 errors.SPECIALenabled. HoldingXand pressingSpacedisplayed all four mapped characters (ẋ,×,ˣ,ₓ) without clipping or scrolling.Fcharacters remained stationary while cycling through the selection.