Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ docs/dist/
.playwright-mcp/
.DS_Store
*.tsbuildinfo
test-results/
playwright-report/
12 changes: 12 additions & 0 deletions docs/__tests__/rendering.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Component rendering fixtures</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="./rendering.ts"></script>
</body>
</html>
1 change: 1 addition & 0 deletions docs/__tests__/rendering.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "../../tests/browser/fixtures/rendering";
57 changes: 57 additions & 0 deletions docs/reviews/component-rendering.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Component rendering review

## Confirmed defects and plan

Baseline: PR #32 at `9c671aca473e2b6e38ea03f2cd74177074fa750d`.
Reviewed the rendered docs at 1280, 768, 390, and 320px before changing component code.
Compared existing bevels, surfaces, typography, and control density with WorkbenchOS.

| Finding | Baseline evidence | Fix |
| -------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- |
| SegmentedMeter overstates partial capacity | At 320px, a 50/100 value fills the entire 257px inner track. Zero-value segments leave a 1px divider. | Size segments against the resolved maximum, leave unused capacity visible, and omit zero-value spans. Use an inset divider that does not consume capacity. |
| Tabs ignore vertical orientation and overflow with long labels | Vertical tabs render horizontally. A 280px label escapes 259px of available content, widening the document to 324px. | Add scoped orientation layout and constrain labels. Preserve nested tab orientation and keyboard handling. |
| Disabled tabs look enabled | Base UI sets data-disabled and aria-disabled, while the CSS only matches native :disabled. | Match the actual state attribute for disabled styling and hover exclusion. |
| Toolbar groups overflow narrow containers | A button and input produce a 301px scroll width inside a 204–216px toolbar. | Let groups wrap and controls shrink within their toolbar; stack vertical groups. |

The plan was to reproduce these defects, make scoped fixes, then check rendered results,
keyboard behavior, neighboring controls, package output, and responsive containment.
The existing maximum normalization, numeric ARIA values, public props, and exports remain
unchanged. Disabled tabs remain focusable without becoming selected.

## Docs re-review and correction

The first implementation also added white inset tab panels and prominent edge-case demos.
Those were poor documentation choices: artificial labels and placeholder content added
clutter, while the inset surface made static content resemble an input. Docs copy alone
was not sufficient justification for changing every consumer's panel appearance.

| Area reviewed | Final decision |
| ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Tabs | Remove the added vertical, nested, and long-label demos from the main page. Restore the original undecorated panel and horizontal spacing. Keep one existing example, mark Advanced disabled, and correct the intro. Retain the functional orientation, wrapping, disabled, and focus fixes. |
| Meter | Remove the added capacity comparison and empty-track demo. Show partial capacity in the existing segmented-meter example: 34 of 40 miles surveyed. Keep the ordinary Meter example for its distinct API. |
| Toolbar | Remove the added narrow and vertical demos. Keep the existing toolbar with one short note about wrapping and orientation. |
| Autocomplete and Fieldset | Keep grouped suggestions and the inherited disabled Switch in existing examples. They demonstrate useful behavior without adding another demo card. |
| Select, RadioGroup, Slider, Accordion, ToggleGroup, Empty, and overlay titles | Keep the brief API guidance and changes inside existing examples. These explain callback types, slot content, or precedence without repeating the component. |

Move the synthetic meter, tab, and toolbar cases to `tests/browser/fixtures/rendering.tsx`.
The browser tests load them through a development-only HTML entry under `docs/__tests__`.
The main docs do not import them, and the production build still uses only `index.html`.
Remove the superseded screenshot gallery so it does not present the reverted design as
an approved result. The original captures remain in the earlier PR commit.

## Validation

- Inspected the simplified docs at 1280, 768, 390, and 320px: tabs, toolbar, meters,
fields, selection controls, grouped controls, application patterns, and overlays.
The public docs have no horizontal overflow. Tab selection still works.
- `npm run check`: formatting, lint, typecheck, and all 54 unit tests pass.
- `npm run test:browser`: all 23 tests pass, including 12 isolated rendering/interaction
regressions and the existing public-docs, window, and overlay checks.
- `npm run build`, `npm run build:docs`, and `npm run pack:check`: pass.
- A separate clean docs build under `/tmp/grayui-simplified-docs-build` confirms that
neither the development entry nor the synthetic fixture content ships in production.
The managed checkout retained unused assets from a previous build; the generated
index references the current assets.
- Browser coverage is Chromium only, using the temporary executable described in the
TypeScript review. No dependency was added. npm emits the environment's existing
unknown `http-proxy` configuration warning.
72 changes: 72 additions & 0 deletions docs/reviews/typescript-component-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# TypeScript and component API review

Reviewed all 43 component modules, the public barrel, declaration build, package exports,
fieldset context, store shims, docs examples, and validation configuration against
`c56271ce216bbd3d7603359869ad1c23ee127030` (0.6.2). The compiler already enables `strict`,
`noUncheckedIndexedAccess`, and `exactOptionalPropertyTypes`; the actionable type defects
were in wrapper contracts and consumer coverage.

## Scope

| Area | Modules inspected |
| ------------------------ | --------------------------------------------------------------------------------------------------------------------- |
| Form controls | checkbox, checkbox-group, date-picker, field, fieldset, input, input-group, number-field, radio-group, select, switch |
| Selection and disclosure | accordion, autocomplete, collapsible, combobox, slider, tabs, toggle-button, toggle-group |
| Overlays | alert-dialog, context-menu, dialog, layer, menu, popover, toast, tooltip |
| Layout and composition | breadcrumbs, button, group-box, scroll-area, separator, table, toolbar, window |
| Status and content | badge, banner, empty, loader, meter, pagination, progress, segmented-meter |

The review examined generic inference, inherited native props, refs, slot forwarding,
controlled and uncontrolled state, disabled-state propagation, accessible names, portal
ownership, emitted declarations, and package consumption. Source inspection covered every
module; focused regressions target the confirmed findings below.

## Validated findings and fixes

| Finding | Evidence before the fix | Change and regression coverage |
| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| Select erases value and multiplicity types | String-only options still accept numeric values and single/array mismatches; callbacks receive `unknown`. A correctly annotated callback is rejected. | Instantiate Base UI with `string` and a multiplicity parameter. Compile positive and negative single/multiple cases, and check multiple-value form submission. Keep the default `SelectProps` prop bag compatible with existing multiple-select annotations. |
| RadioGroup erases its string option contract | Numeric selections compile although every rendered option has a string value; callbacks receive `unknown`. | Instantiate the primitive's string props. Compile callback inference and reject numeric selections. |
| Slider, Accordion, and ToggleGroup erase generic information | A scalar Slider callback receives a number/array union; Accordion loses its item type; ToggleGroup widens literal unions to `string`. | Carry the primitive type parameter through each wrapper. Compile number/range, string-array, literal-union, and Slider ref examples. |
| Grouped Autocomplete loses item inference and calls a component directly | Grouped object callbacks infer `unknown`. The wrapper invokes the Base UI root as an ordinary function. | Restore grouped and flat overloads, retain the broad props overload for forwarding, and render the primitive through JSX. Compile grouped/flat consumers and exercise grouped filtering and selection in the docs. |
| Popup titles accidentally intersect with the native `title` attribute | React elements are rejected for Dialog, AlertDialog, and Popover titles despite the explicit `ReactNode` declaration. | Omit native `title` before declaring the content slot. Compile all three and verify dialog accessible names with React element titles. |
| Empty silently discards children | `<Empty title="…">Create a file</Empty>` renders no action content. | Render children in the existing contents slot when `contents` is undefined. Preserve explicit contents precedence. Check DOM output and the docs action button. |
| Switch ignores greyUI's disabled fieldset context | A switch inside nested disabled fieldsets fires its callback and lacks disabled state on the control and label. The docs reproduction fails at all four viewport widths. | Resolve inherited disabled state through the same context used by Checkbox. Test nested fieldsets, explicit `disabled={false}`, blocked interaction, and the enabled control. |
| Package checks do not compile consumer contracts | The original declaration build and package audit pass despite the API type defects above. | Compile the same consumer examples against source paths during `typecheck` and real root/subpath package exports during `pack:check`, with `skipLibCheck: false` for published declarations. |
| Browser output contaminates subsequent checks | `npm run check` attempts to format generated Playwright JSON and trace HTML after browser tests. | Ignore `test-results/` and `playwright-report/`, then run the normal checks after browser validation. |

Type parameter preservation follows the [TypeScript generics guidance](https://www.typescriptlang.org/docs/handbook/2/generics.html).
The Autocomplete rendering change follows React's rule to
[let React call components through JSX](https://react.dev/reference/rules/react-calls-components-and-hooks#never-call-component-functions-directly).
The existing, documented assertion bridging Base UI's overloads remains confined to that
implementation boundary.

## Validation

- Baseline `npm run check`: 51 tests pass. New consumer fixtures fail on the original
value/callback/title/grouped-item contracts; the Empty and disabled Switch regressions
fail before implementation changes.
- Final `npm run check`: formatting, lint, TypeScript, and 54 tests pass.
- `npm run build`, `npm run build:docs`, and `npm run pack:check` pass. Package validation
covers strict consumer declaration checking, entrypoint existence, SSR imports, runtime
externals, and bundle budgets.
- Browser suite: 11 tests pass, including existing window and overlay regressions and new
component API flows at 1280, 768, 390, and 320 pixels.
- Reviewed rendered captures of grouped controls, open suggestions, Empty children, and a
dialog with a React title. Compared original and fixed component code using the updated
docs examples. The original code reproduces the missing disabled Switch state at all
four widths; the fixed examples and document remain contained.

The interactive cloud browser could not open localhost and the standard Playwright browser
CDN timed out. Local browser tests used a temporary npm-distributed Chromium executable via
`PLAYWRIGHT_EXECUTABLE_PATH`; no browser dependency or executable is added to the package.
Validation emitted environment warnings about npm's `http-proxy` setting and conflicting
color-output variables. Cross-browser and assistive-technology testing were not performed.

## Compatibility notes

Select and RadioGroup now reject non-string selections that cannot match their string-valued
options. JSX Select usage also rejects mismatched single and multiple value shapes. For a
precisely typed prop bag, use `SelectProps<false>` or `SelectProps<true>`; the unparameterized
type continues to allow both modes. Existing string-valued controls, explicit Empty contents,
flat Autocomplete items, and popup string titles remain supported.
Loading
Loading