refactor(ui): open the remaining option menus from PickerSurface - #205
Merged
Conversation
Four dropdowns positioned themselves. Each measured its anchor with its own getBoundingClientRect block and painted a `surface-float fixed z-9999` div, so each also missed what PickerSurface already does: re-measuring while the form panel scrolls, flipping above the trigger when there is no room below, and becoming a bottom sheet on Android. The keychain's KeySelector was the furthest gone. Its local DropdownItem was a copy of PickerOption down to the class list and the accent check, DropdownDivider a copy of PickerDivider, and its trigger a copy of PickerTrigger including the 150ms chevron rotation. It also carried a hand-rolled flip that guessed the menu height from a row count. All of it is deleted in favour of the shared parts. KeychainToolbar, VaultPicker and PortInput already used the canonical rows, so they only lose their positioning code. PickerSurface grows what those callers needed: `width: "content"` for a menu that sizes to its rows rather than to a narrow trigger (pinned by `right` when right-aligned, since there is no width to subtract from the anchor's edge), `minWidth` as the floor for that mode, and `maxHeight` for the port list's shorter cap. Left alone deliberately: SyncDropdown and ShareMenu are anchored panels with a header, a fixed width and an entry animation rather than option lists, and SidebarAccountButton compensates for the UI scale with its own transform.
`min-w-52` on the vault picker's old surface was 13rem, and rem tracks the UI scale: at the default scale the root is 15px, so the floor was 195px, not the 208px the port count assumed. A before/after capture put the converted menu 13px wider than the one it replaced. `minWidth` now takes any CSS length and the caller passes `13rem`, which restores the old width exactly and keeps it correct at every UI scale.
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
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.
Follow-up to #203. Four more dropdowns positioned themselves: each measured its
anchor with its own
getBoundingClientRectblock and painted asurface-float fixed z-9999div. Each therefore also missed whatPickerSurfacealready does — re-measuring while the form panel scrolls, flipping above the
trigger when there is no room below, and becoming a bottom sheet on Android.
keychain/IdentityForm'sKeySelectorwas the furthest gone. Its localDropdownItemwas a copy ofPickerOptiondown to the class list and the accentcheck,
DropdownDividera copy ofPickerDivider, and its trigger a copy ofPickerTriggerincluding the 150ms chevron rotation. It also carried ahand-rolled flip that guessed the menu height from a row count. All of that is
deleted in favour of the shared parts.
KeychainToolbar,VaultPickerandPortInputalready used thecanonical rows, so they only lose their positioning code.
PickerSurfacegrows what those callers needed:width: "content"— a menu that sizes to its rows rather than to a narrowtrigger. Pinned by
rightwhen right-aligned, since there is no width tosubtract from the anchor's edge.
minWidth— the floor for that mode, soVaultPickerkeeps itsmin-w-52offa text-sized trigger.
maxHeight—PortInput's list keeps its shorter 240px cap.Left alone deliberately
SyncDropdownandShareMenuare anchored panels — header row, close button,fixed
w-64/280px,animate-fadeInwithtransformOriginmath — not optionlists; routing them through
PickerSurfacewould be a redesign.SidebarAccountButtoncompensates for the UI scale with its owntransform: scale(uiScale), whichPickerSurfacehas no handling for.connections/KeySelectoris intentionally still separate from the keychain one:that one picks between an inline key and the keychain and ends in a "manage in
keychain" action, this one has a third "no key" state and no key-type badges.
Unifying them would take more props than the two compositions cost lines, and
there is now a comment saying so.
Verification
tsc --noEmitclean; 132 tests acrossshared,connectionsandkeychainpass (266 including
settings).Driven in the headless build and read back from screenshots:
KeychainToolbar— right-aligned, content-width, rows unchanged;IdentityForm's key selector — sits at the bottom of the form panel and nowflips above the trigger, with the accent check on "No key";
VaultPicker— content width at its 208px floor, left-aligned to the smalltext trigger;
PortInput— against two emulated ptys, the list opens under the input,picking one fills
/dev/ttyUSB99and dismisses the surface.