The drawer's trailing glyphs share one column, and hover actions stop reserving it - #76
Merged
Merged
Conversation
… reserving it Every drawer row right-anchored a different-width box against the same 12px gutter, so the outermost glyph landed at a different x per row type: the PROJECTS refresh at 24px from the panel edge, a band's status dot at 15, a project row's + at 28. Box-edge alignment is not optical alignment — an AbIconButton pads a 14px glyph inside a 24px box and a status dot is a bare 6px circle. AbRowTrailingCell centres either in a cell of the button's own reported footprint, so the column holds at any UI Size and on both platforms; a scalar token would have been right at exactly one of them. Row(spacing:) also charged a 4px gap for the zero-width children that _RemoveButton and DrawerProjectAggregateDot returned when they had nothing to show. Kit assembly drops absent children before layout instead. The hover-only buttons held their width through Visibility(maintainSize:), which reserved both axes to fix a problem that only existed on one: an icon button is the tallest thing in an sm row, so mounting one on pointer-enter grew the row ~10px. AbRowContentFloor anchors the height on the row, which lets the actions collapse — 32-72px per row given back to names, on a 288px panel. session_row.dart already worked this way; its hardcoded 24px leading anchor moves to the same floor, which also stops it jittering at UI Size 1.25, where the scaler-multiplied kebab beat it. Reveal is hover, keyboard focus, or an open confirm dialog. maintainSize excluded semantics but not focus, so the invisible trash and discard buttons were already tab-reachable; collapsing on hover alone would have made them unreachable instead. Ordering is now actions-outermost on every row class. The file tree takes the floor and the collapse but no cell — its terminal element is a variable-width diff-stat badge in a resizable pane with no fixed edge to align to. Comments that cited the old mechanism are rewritten, including file_tree_view.dart's claim to use "the same technique session_row.dart uses", which described the opposite of what session_row.dart does.
…t drops Review of the trailing-rail change turned up four ways a revealed affordance outlives or outlasts the state that owns it. The + now latches the row it was revealed from for as long as its create runs, the way the trash already did: a cold remote open takes tens of seconds, and unmounting mid-flight took the re-entrancy guard with it, so a second hover and a second tap launched a concurrent session and the failure snackbar reported to a dead context. AbListRow only mounts the detector that owns the focus highlight for an enabled, interactive row, and dropping it reports no closing false. A row that goes disabled while focused therefore latched every focus-revealed glyph on with nothing focused. didUpdateWidget now clears the bit and reports it, deferred because the caller answers with setState. AbRowTrailingSwap excluded the resting STATUS glyph from semantics the moment the row was revealed, and reveal is driven by focus as well as hover — so a screen-reader user arriving at a machine band was the one reader who never heard whether it was online. Only the invisible ACTION leaves the tree now. _AdvertisedProjectRow became stateful in the same change but was built unkeyed in a list the control plane reorders live, so positional reconciliation handed one project's focus latch to another. Two subscriptions were being churned rather than held: DrawerProjectAggregateDot.needsUser ran inside HoverableDrawerRow's builder, where ConsumerStatefulElement retires anything the element's own build did not re-read, and _RemoveButton.offersFor sat behind an && whose left side is hover, giving one predicate opposite lifetimes at its two call sites. The file tree floors row height on whether the TREE was wired with git callbacks, not on the platform: the Files tab mounts no buttons at all and was paying a third of a row's height for them. And a kit nested inside another kit no longer claims a rail cell — the panel-edge column belongs to the outer one — which kit() now expresses with ownsColumn instead of leaving to the caller. AbDockedColumn's minBodyExtent scales with the text scaler. The rows it holds room for floor on AbIconButton.boxExtent, so a raw 44 fell short of the FIRST row above UI Size ~1.15 and the drawer's list strip stopped containing a whole one; the restored assertion measures the band's bottom rather than pinning a sliver.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What changed
Every glyph on the right edge of the projects drawer now sits in one optical
column, and the actions that only appear on hover no longer reserve width when
they are not there.
The column is a new design-system primitive,
AbRowTrailingCell: a cell as wideas
AbIconButton.footprintWidth(context)with its tenant centred, so a bare 6pxstatus dot and a 14px glyph padded inside a 24px button box land on the same
centre line.
AbRowTrailingCell.kitassembles a row's trailing from a list thatmay contain nulls, dropping them before layout —
Row(spacing:)charges a gapfor a zero-width child, which is what put a project row's
+4px inboard ofeverything else.
Reservation is separated from reveal by
AbRowContentFloor, which anchors arow's height on a button's box so the buttons themselves can be absent from the
tree entirely.
AbRowContentFloor.noneis the default and emits noConstrainedBoxat all, so the existingAbListRowcall sites arepixel-identical.
Why
Four row classes were computing their trailing edge four different ways, so the
same panel put its glyphs 15, 24, 28 and 44px from the right edge depending on
which kind of row you were looking at. The causes were box-edge alignment
instead of optical, the phantom
Row(spacing:)gap above, and status/actionappearing in opposite orders between row classes.
Separately, hover-only actions were held in
Visibility(maintainSize: true),which reserves both axes to solve a problem that was only ever vertical: the
row jumping ~10px as buttons mounted. The cost was 32-72px of permanent width
per row in a 288px panel, taken straight out of the project name's ellipsis
budget.
Fixing those surfaced four live defects, which are the second commit:
maintainSizeexcludes semantics but not focus, so the invisible trashand discard buttons were already tab-reachable. Reveal is now
hover || focus || in-flight.+is only built while its row is revealed, so a cold remote open — tensof seconds — unmounted the button mid-flight and took its re-entrancy guard
with it: a second hover and a second tap started a concurrent session, and the
failure snackbar reported to a dead context. It latches the row now, as the
trash already did.
AbListRowonly mounts the detector that owns the focus highlight for anenabled, interactive row, and dropping it reports no closing
false— so arow that went disabled while focused latched every focus-revealed glyph on
with nothing focused.
session_row.dartanchored its height on a hardcoded 24, which the textscaler beats: that row jitters on hover today at UI Size 1.25.
How it was verified
Windows, from
app/:flutter analyze— no issues (errors, warnings and infos; not the MCPerrors-only view)
flutter test— 3135 pass, 2 skip (both pre-existing skips)npm run check:font-tokens— cleanNew coverage, each verified to fail against the code it guards:
test/design/widgets/ab_row_trailing_test.dart— thatAbIconButton.footprintWidthstill equals whatAbTapTargetactually laysout, across both platforms x three text scales, since nothing in the type
system holds the two together; that a dot and a button share one centre; and
that a dropped cell buys no gap.
test/widgets/drawer_rail_test.dart— the rail measured end to end acrossevery row class in a real drawer, keyboard reveal, and a revealed action
outliving the hover its own modal ends.
test/design/widgets/ab_list_row_test.dart—onFocusChangereports thefocus it drops rather than latching.
No workspace outside
app/is touched, so no bridge/relay/web suites were run.Screenshots
Before: the drawer's right edge, with the four different trailing positions the
first commit collapses into one — in the issue thread rather than here. No
after-capture yet; the geometry is pinned by
drawer_rail_test.dartinstead,which measures the centre of every trailing glyph in a mounted drawer against a
single rail.
Checklist
development, notmaindocumented behaviour changes