Skip to content

fix(ui): close package-wide gaps from the component library PRs - #1047

Open
EhabY wants to merge 2 commits into
mainfrom
feat/ui-package-gaps
Open

fix(ui): close package-wide gaps from the component library PRs#1047
EhabY wants to merge 2 commits into
mainfrom
feat/ui-package-gaps

Conversation

@EhabY

@EhabY EhabY commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

🤖 This PR was created by Coder Agents on behalf of @EhabY.

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/ui declares its Storybook devDependencies; the root storybook pin moves into the pnpm catalog.
  • Tokens: secondary-button colors, --ui-font-size-small, --ui-z-index-overlay, --ui-panel-background for 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.
  • Button matching monaco-text-button, with a secondary variant; the state panels and story triggers use it instead of faking one.
  • LoadingState panel reusing the StatePanel skeleton stylesheet, with the large Spinner in the icon slot.
  • Menus gain *CheckboxItem, *RadioGroup/*RadioItem, *Label, and *Keybinding, styled like native. Both menus build their styled parts from one shared menuPart factory: the Radix packages create separate menu scopes so the components cannot be shared, but their part props are the same Menu*Props types, so the factory keeps the public types exact and the React Compiler still memoizes what it produces.
  • Keybinding hints take a contribution's key/mac/win/linux fields and render the current OS's binding in the native label style (⇧⌘R on macOS, Ctrl+Shift+R elsewhere); formatKeybinding is exported for other surfaces. key is required and the platform fields are overrides, matching VS Code's contributes.keybindings schema, so the formatter is total. VS Code has no API to resolve user-remapped bindings, so hints show contributed defaults (documented known gap).
  • useVscodeTheme re-renders on same-kind theme switches: one shared observer, and a snapshot of theme kind plus id that the returned kind is derived from.
  • Reduced motion keeps activity cues: striped indeterminate ProgressBar band, Spinner arc over a tinted ring.
  • Button parity fixes surfaced by the vscode-elements comparison: focus outlines apply on :focus like native (a click shows the ring; workbench style.css outlines button:focus), a toggled IconButton keeps its active background on hover (native applies checked colors as inline styles in toggle.ts), and IconButton no longer defaults a browser title box (native hints via the styled hover widget; wrap in Tooltip).
  • The toolbar button's focus ring now wins over its hover and toggled rings. All three share one outline, so the toggled rule was hiding the ring; natively they never compete, because the ring is an outline while a toggle's active state is an inline borderColor. :where() keeps the exclusions weightless so source order alone decides.
  • TooltipProvider becomes 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. Tooltip has no per-instance delay prop; nest a provider where a different delay is needed.
  • Layout follows StatePanel's precedent: the shared menu module is components/Menu/Menu.tsx with its stylesheet beside it, so components/ holds one folder per component plus the control.css and overlay.css bases, and the keybinding.ts formatter sits with the other package-level modules.

Testing

pnpm typecheck, lint, format:check, test (2222 passed), and storybook:ci pass.

Values were checked against current VS Code source rather than inferred: .monaco-menu uses cornerRadius-large, .action-menu-item and .monaco-action-bar .action-label use cornerRadius-medium, the workbench hover pins a literal 5px radius with 19px leading and a 700px cap, .monaco-menu is min-width: 160px with padding: 0 2em labels and a fadeIn 0.083s, and style.css outlines plain button:focus with outline-offset: -1px while button is deliberately absent from its :active { outline: 0 } list.

Every component has a story, at all four captured themes; StatePanel and Menu are 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 secondary Button and 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-motion and forced-colors branches 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 from forced-colors: active. Pixel CI will show intended diffs where stories now use Button as the overlay trigger and where the context menu and loading panel render more rows.

@EhabY EhabY self-assigned this Jul 28, 2026
@EhabY
EhabY force-pushed the feat/ui-package-gaps branch 2 times, most recently from 50045c8 to 35ba10d Compare July 28, 2026 18:09
…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
EhabY force-pushed the feat/ui-package-gaps branch from 35ba10d to c7371be Compare July 28, 2026 18:23
@EhabY
EhabY requested a review from jakehwll July 29, 2026 07:40
…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
EhabY force-pushed the feat/ui-package-gaps branch from 33d9497 to 93c06c1 Compare July 29, 2026 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ui: track package-wide gaps deferred from the component library PRs

1 participant