fix(ui): close package-wide gaps from the component library PRs - #1047
Open
EhabY wants to merge 2 commits into
Open
fix(ui): close package-wide gaps from the component library PRs#1047EhabY wants to merge 2 commits into
EhabY wants to merge 2 commits into
Conversation
8 tasks
EhabY
force-pushed
the
feat/ui-package-gaps
branch
2 times, most recently
from
July 28, 2026 18:09
50045c8 to
35ba10d
Compare
…ary PRs Closes #1046. - Declare Storybook devDependencies; pin storybook via the pnpm catalog - Add Button matching monaco-text-button, with a secondary variant - Add LoadingState reusing the StatePanel skeleton - Tokens: secondary-button colors, small type tracking bodyFontSize-xSmall, overlay z-index, VS Code's cornerRadius and spacing scales, and --ui-panel-background for editor-tab and panel hosts - Re-render useVscodeTheme on same-kind theme switches (one shared observer) - Keep reduced-motion activity cues: striped indeterminate band, tinted ring - Menus: checkbox/radio/label/keybinding wrappers; keybinding hints render the current OS's binding in the native label style (formatKeybinding) - Tooltip: app-level TooltipProvider so trigger-to-trigger hovers skip the delay; tooltips cap at half the window height like native
EhabY
force-pushed
the
feat/ui-package-gaps
branch
from
July 28, 2026 18:23
35ba10d to
c7371be
Compare
…cus ring Build the styled menu parts from a `menuPart` factory instead of two near-identical files. The Radix packages create separate menu scopes, so components cannot be shared, but their part props are the same `Menu*Props` types, so the factory keeps the public types exact: wrong-part and unknown props are still rejected, and the React Compiler still memoizes the component it produces. `Content` and `SubContent` stay explicit, their defaults differ. Following StatePanel's precedent the shared module becomes `components/Menu/Menu.tsx` with its stylesheet beside it, leaving `components/` holding only the `control.css` and `overlay.css` bases; `keybinding.ts` is a formatter rather than a component, so it sits with the other package-level modules. Require `key` in the object form of `Keybinding`, matching VS Code's `contributes.keybindings` schema where the platform fields are overrides. The all-undefined case disappears, so `formatKeybinding` no longer needs an empty-string escape hatch. Drop a provably dead branch in `keyLabel` and the `Set` plus mutable accumulator in `formatChord`, and derive the theme kind from the store snapshot rather than subscribing for the re-render and reading the DOM again. Order the toolbar button's outline rules so focus wins over the hover and toggled rings, with `:where()` keeping the exclusions weightless. VS Code outlines `button:focus` through a dedicated `outline` while a toggle's active state is an inline `borderColor`, so the ring natively stays visible on a pressed or hovered button; ours hid it. This is the only rendering change. Re-checked the overlay, menu, and button metrics against VS Code: the radius tokens and the literal hover values are faithful, so no other declaration moved, and the reduced-motion and forced-colors branches are untouched. Cut comments that restated their code, keeping one line of JSDoc per public component, and drop the nine spacing tokens that had no call sites, with the token policy now stated in the README. Story assertions that restated the props a story had just passed are gone, the menu-opening steps are shared through `#storybook`, and the context menu and loading panel now render their remaining visual cases. Unmount before the theme test clears its attributes: vitest runs afterEach in reverse registration order, so the reset ran before Testing Library's cleanup and the observer could update a mounted hook outside act().
EhabY
force-pushed
the
feat/ui-package-gaps
branch
from
July 29, 2026 09:10
33d9497 to
93c06c1
Compare
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.
Closes #1046 (all 8 checkboxes) plus the overlays gaps deferred from #1039; list/selection-row tokens stay deferred to the Tree suite (#1037).
Changes
packages/uideclares its Storybook devDependencies; the rootstorybookpin moves into the pnpm catalog.--ui-font-size-small,--ui-z-index-overlay,--ui-panel-backgroundfor non-sidebar hosts, and VS Code's cornerRadius and spacing scales. Only the spacing rungs components actually use are declared, and the README states the policy for adding more. Metrics VS Code hardcodes natively (button, hover, and menu paddings, the 5px hover radius) stay hardcoded.Buttonmatchingmonaco-text-button, with asecondaryvariant; the state panels and story triggers use it instead of faking one.LoadingStatepanel reusing theStatePanelskeleton stylesheet, with the largeSpinnerin the icon slot.*CheckboxItem,*RadioGroup/*RadioItem,*Label, and*Keybinding, styled like native. Both menus build their styled parts from one sharedmenuPartfactory: the Radix packages create separate menu scopes so the components cannot be shared, but their part props are the sameMenu*Propstypes, so the factory keeps the public types exact and the React Compiler still memoizes what it produces.key/mac/win/linuxfields and render the current OS's binding in the native label style (⇧⌘Ron macOS,Ctrl+Shift+Relsewhere);formatKeybindingis exported for other surfaces.keyis required and the platform fields are overrides, matching VS Code'scontributes.keybindingsschema, so the formatter is total. VS Code has no API to resolve user-remapped bindings, so hints show contributed defaults (documented known gap).useVscodeThemere-renders on same-kind theme switches: one shared observer, and a snapshot of theme kind plus id that the returned kind is derived from.ProgressBarband,Spinnerarc over a tinted ring.:focuslike native (a click shows the ring; workbenchstyle.cssoutlinesbutton:focus), a toggledIconButtonkeeps its active background on hover (native applies checked colors as inline styles intoggle.ts), andIconButtonno longer defaults a browsertitlebox (native hints via the styled hover widget; wrap inTooltip).outline, so the toggled rule was hiding the ring; natively they never compete, because the ring is anoutlinewhile a toggle's active state is an inlineborderColor.:where()keeps the exclusions weightless so source order alone decides.TooltipProviderbecomes a public export mounted once per app, so hovers moving between triggers skip the 500ms delay like native; tooltips cap at half the window height.Tooltiphas no per-instance delay prop; nest a provider where a different delay is needed.StatePanel's precedent: the shared menu module iscomponents/Menu/Menu.tsxwith its stylesheet beside it, socomponents/holds one folder per component plus thecontrol.cssandoverlay.cssbases, and thekeybinding.tsformatter sits with the other package-level modules.Testing
pnpm typecheck,lint,format:check,test(2222 passed), andstorybook:cipass.Values were checked against current VS Code source rather than inferred:
.monaco-menuusescornerRadius-large,.action-menu-itemand.monaco-action-bar .action-labelusecornerRadius-medium, the workbench hover pins a literal 5px radius with 19px leading and a 700px cap,.monaco-menuismin-width: 160pxwithpadding: 0 2emlabels and afadeIn 0.083s, andstyle.cssoutlines plainbutton:focuswithoutline-offset: -1pxwhilebuttonis deliberately absent from its:active { outline: 0 }list.Every component has a story, at all four captured themes;
StatePanelandMenuare internal. Stories cover the button variants and disabled state, toolbar button default/toggled/disabled, both state panels, all pill tones plus the native badge, spinner sizes, determinate and indeterminate progress, search input filled and disabled, tooltip open and overflowing, and both menus with items, disabled rows, keybinding hints, separators, checkbox, label, radio group, submenu, and a scrolling long menu. The parity story compares the secondaryButtonand an open menu against vscode-elements.Three visual cases are knowingly uncovered. Hover, focus, and pressed rings are not snapshotted, because every play function restores the resting state to keep snapshots deterministic, so the focus-ring change above is verified against VS Code source rather than by pixel. The
prefers-reduced-motionandforced-colorsbranches are not reachable from a story, since Pixel's matrix covers themes, color schemes, browsers, and viewports but not emulated media. High-contrast themes are covered and are a different thing fromforced-colors: active. Pixel CI will show intended diffs where stories now useButtonas the overlay trigger and where the context menu and loading panel render more rows.