diff --git a/.changeset/menu-context-menu-api-reference.md b/.changeset/menu-context-menu-api-reference.md new file mode 100644 index 00000000..3635ba9a --- /dev/null +++ b/.changeset/menu-context-menu-api-reference.md @@ -0,0 +1,29 @@ +--- +"@makeplane/propel": minor +--- + +Standardize the Menu and ContextMenu APIs and the generated props table. + +Rows in both families (`MenuItem`, `MenuCheckboxItem`, `MenuRadioItem`, `MenuLinkItem`, +`MenuSubmenuTrigger` and their `ContextMenu*` counterparts) rename `tone` to `variant`, following +the button-family rework — one look axis, same value spellings, now defaulting to `neutral` so the +prop is optional. `MenuItemTone` / `ContextMenuItemTone` / `ContextMenuSubmenuTriggerTone` become +`MenuItemVariant` / `ContextMenuItemVariant` / `ContextMenuSubmenuTriggerVariant`. The rows' +`endContent` prop is renamed `trailing`, and the styled part `MenuItemEndContent` is renamed +`MenuItemTrailing`. The `elements` menu row `layout` axis renames its values from `default` / +`with-description` to `single` / `stacked`. + +Fixes `MenuItem` dropping a consumer-supplied `render`: the ready-made hard-coded its own render +target after spreading props, so `} />` silently rendered a `
`. +The prop is now threaded into the styled row and typed to what it actually accepts. Every other +ready-made that hard-codes its render target (`MenuContent`, `MenuLabel`, `MenuSeparator`, the +remaining rows, and the `ContextMenu` equivalents) now omits `render` instead of advertising a prop +it ignores. + +Corrects two wrong entries the props table was advertising: `MenuContent`'s `sizing` claimed an +`"anchor"` default it never applied (omitting it hugs the content), and `MenuSubmenuContent` +inherited `MenuContent`'s `side` default of `"bottom"` when it actually opens to the `"right"`. +`ContextMenuContent`'s positioning props are documented per position rather than with a single +default, since the same component anchors to the pointer at the root and beside its parent row in a +submenu. Every Menu and ContextMenu prop now carries a description and, where a real default +exists, a `@default` tag. diff --git a/apps/docs/src/components/PropsTable.astro b/apps/docs/src/components/PropsTable.astro index ab94e35b..d9d2c9ee 100644 --- a/apps/docs/src/components/PropsTable.astro +++ b/apps/docs/src/components/PropsTable.astro @@ -1,11 +1,15 @@ --- -import type { PropItemType } from "react-docgen-typescript"; +import type { PropItem, PropItemType } from "react-docgen-typescript"; import { getComponentDoc } from "~/lib/props-schema"; type Part = { + /** Path relative to `packages/propel/src`, e.g. `"components/menu/menu-item.tsx"`. */ source: string; + /** The exported component name to document, e.g. `"MenuItem"`. */ component: string; + /** Extra prop names to omit from this part's table, on top of the always-hidden set. */ + hiddenProps?: string[]; }; type Props = { @@ -14,6 +18,9 @@ type Props = { const { parts } = Astro.props; +// Inherited React internals that appear on every component and describe none of them. +const ALWAYS_HIDDEN = ["key", "ref"]; + // `shouldExtractLiteralValuesFromEnum` (props-schema.ts) puts a string-literal union's members in // `type.value` (each `{ value: '"primary"' }`) while `type.name` stays the generic `"enum"` — join // them back into the `"primary" | "secondary" | …` form so the table shows the real union, not the @@ -24,44 +31,98 @@ function formatType(type: PropItemType): string { } return type.name; } + +// react-docgen-typescript reads the `@default` tag verbatim, so a string default arrives already +// quoted (`"start"`) and a boolean/number bare (`false`, `4`) — render it as-is rather than +// re-quoting by type, which would turn `4` into `"4"`. +function formatDefault(prop: PropItem): string | null { + const value = prop.defaultValue?.value; + return value == null || value === "" ? null : String(value); +} + +// One shape for every table: the props a consumer MUST pass, then the ones they may, each group +// keeping its declaration order. Without this the order is whatever the intersection type happened +// to produce, which puts inherited props like `render` ahead of the component's own required props. +function orderProps(props: PropItem[]): PropItem[] { + return [...props.filter((prop) => prop.required), ...props.filter((prop) => !prop.required)]; +} ---
{ - parts.map(({ source, component }) => { + parts.map(({ source, component, hiddenProps = [] }) => { const doc = getComponentDoc(source, component); - if (!doc || Object.keys(doc.props).length === 0) { + if (!doc) { + // A renamed or moved export silently produced an empty table before; surface it on the page + // so a broken reference cannot ship unnoticed. console.warn( - `[PropsTable] No component doc found for "${component}" at source "${source}" — the props table will be empty. Check the source path (relative to packages/propel/src) and the exported component name.`, + `[PropsTable] No component doc found for "${component}" at source "${source}". Check the source path (relative to packages/propel/src) and the exported component name.`, + ); + return ( +
+

+ {component} was not found at{" "} + {source}. The export was probably renamed or moved — + update this page's parts list. +

+
); } - const props = doc ? Object.values(doc.props) : []; + + const hidden = new Set([...ALWAYS_HIDDEN, ...hiddenProps]); + const props = orderProps(Object.values(doc.props).filter((prop) => !hidden.has(prop.name))); + return (
-

{component}

+

{component}

+ {doc.description ? ( +

{doc.description}

+ ) : null}
- - - - - - - - - - - {props.map((prop) => ( - - - - - + {props.length === 0 ? ( +

+ No component-specific props — accepts the standard HTML attributes for the element it + renders. +

+ ) : ( +
PropTypeRequiredDescription
{prop.name} - {formatType(prop.type)} - {prop.required ? "Yes" : "No"}{prop.description}
+ + + + + + - ))} - -
PropTypeDefaultDescription
+ + + {props.map((prop) => { + const defaultValue = formatDefault(prop); + return ( + + + {prop.name} + {prop.required ? ( + <> + + (required) + + ) : null} + + + {formatType(prop.type)} + + + {defaultValue ?? "—"} + + {prop.description} + + ); + })} + + + )}
); diff --git a/apps/docs/src/demos/context-menu/basic.tsx b/apps/docs/src/demos/context-menu/basic.tsx index 91615fbf..82370950 100644 --- a/apps/docs/src/demos/context-menu/basic.tsx +++ b/apps/docs/src/demos/context-menu/basic.tsx @@ -15,28 +15,25 @@ export default function BasicDemo() { }>Right-click here } aria-keyshortcuts="Meta+X" - endContent={} + trailing={} label="Cut" /> } aria-keyshortcuts="Meta+C" - endContent={} + trailing={} label="Copy" /> } aria-keyshortcuts="Meta+V" - endContent={} + trailing={} label="Paste" /> - } label="Delete" /> + } label="Delete" /> ); diff --git a/apps/docs/src/demos/context-menu/link-items.tsx b/apps/docs/src/demos/context-menu/link-items.tsx index 268d6048..8be22f56 100644 --- a/apps/docs/src/demos/context-menu/link-items.tsx +++ b/apps/docs/src/demos/context-menu/link-items.tsx @@ -16,25 +16,18 @@ export default function LinkItemsDemo() { }>Right-click here } href="#work-item" target="_blank" rel="noopener noreferrer" label="Open in new tab" /> - } - href="#activity" - label="Go to activity" - /> + } href="#activity" label="Go to activity" /> } aria-keyshortcuts="Meta+C" - endContent={} + trailing={} label="Copy link" /> diff --git a/apps/docs/src/demos/context-menu/selection-rows.tsx b/apps/docs/src/demos/context-menu/selection-rows.tsx index 7f3d792a..284e9560 100644 --- a/apps/docs/src/demos/context-menu/selection-rows.tsx +++ b/apps/docs/src/demos/context-menu/selection-rows.tsx @@ -18,7 +18,6 @@ export default function SelectionRowsDemo() { }>Right-click for view options - - + + diff --git a/apps/docs/src/demos/context-menu/submenu.tsx b/apps/docs/src/demos/context-menu/submenu.tsx index d1cdde8f..ff756755 100644 --- a/apps/docs/src/demos/context-menu/submenu.tsx +++ b/apps/docs/src/demos/context-menu/submenu.tsx @@ -15,22 +15,18 @@ export default function SubmenuDemo() { }>Right-click here - } label="Rename" /> - } label="Duplicate" /> + } label="Rename" /> + } label="Duplicate" /> - } - label="Move to" - /> + } label="Move to" /> - - - + + + - } label="Delete" /> + } label="Delete" /> ); diff --git a/apps/docs/src/demos/dialog/open-from-menu.tsx b/apps/docs/src/demos/dialog/open-from-menu.tsx index b0ff77d6..2eeb5124 100644 --- a/apps/docs/src/demos/dialog/open-from-menu.tsx +++ b/apps/docs/src/demos/dialog/open-from-menu.tsx @@ -39,7 +39,7 @@ export default function OpenFromMenuDemo() { } label="Copy link" /> } onClick={() => setOpen(true)} label="Delete project…" diff --git a/apps/docs/src/demos/home/members-table.tsx b/apps/docs/src/demos/home/members-table.tsx index c7525ef7..263ba436 100644 --- a/apps/docs/src/demos/home/members-table.tsx +++ b/apps/docs/src/demos/home/members-table.tsx @@ -64,7 +64,7 @@ export default function MembersTableDemo() { } label="Edit" /> - } label="Delete" /> + } label="Delete" /> diff --git a/apps/docs/src/demos/menu/basic.tsx b/apps/docs/src/demos/menu/basic.tsx index 5eb56b69..7296f69a 100644 --- a/apps/docs/src/demos/menu/basic.tsx +++ b/apps/docs/src/demos/menu/basic.tsx @@ -24,7 +24,7 @@ export default function BasicDemo() { } label="Make a copy" /> } label="Open in new tab" /> - } label="Delete" /> + } label="Delete" /> ); diff --git a/apps/docs/src/demos/menu/submenu.tsx b/apps/docs/src/demos/menu/submenu.tsx index 6f43ef57..7518e0ca 100644 --- a/apps/docs/src/demos/menu/submenu.tsx +++ b/apps/docs/src/demos/menu/submenu.tsx @@ -34,7 +34,7 @@ export default function SubmenuDemo() { } + trailing={} label="Priority" /> @@ -62,7 +62,7 @@ export default function SubmenuDemo() { } + trailing={} label="State" /> diff --git a/apps/docs/src/demos/table/rich-rows.tsx b/apps/docs/src/demos/table/rich-rows.tsx index bd2fc79b..c9caf10b 100644 --- a/apps/docs/src/demos/table/rich-rows.tsx +++ b/apps/docs/src/demos/table/rich-rows.tsx @@ -67,7 +67,7 @@ export default function RichRowsDemo() { } label="Edit" /> - } label="Delete" /> + } label="Delete" /> diff --git a/apps/docs/src/lib/props-schema.ts b/apps/docs/src/lib/props-schema.ts index e161f42a..07d3b506 100644 --- a/apps/docs/src/lib/props-schema.ts +++ b/apps/docs/src/lib/props-schema.ts @@ -18,8 +18,17 @@ const propelTsconfig = resolve(propelRoot, "tsconfig.json"); const parser = withCustomConfig(propelTsconfig, { shouldExtractLiteralValuesFromEnum: true, shouldRemoveUndefinedFromOptional: true, + // A prop propel declares itself is public API even when it SHADOWS a native attribute of the + // same name (`placeholder`, `value`, `type`). docgen collapses those into one entry whose + // `parent` is whichever declaration it saw first — @types/react — so a parent-only test drops + // them: `MenuSearch`'s documented `placeholder` was invisible this way. `declarations` lists + // every declaration site, so a prop survives when ANY of them is outside node_modules; the ~300 + // purely-inherited DOM/React attributes have none and are still filtered. propFilter: (prop) => - prop.name === "children" || !prop.parent || !prop.parent.fileName.includes("node_modules"), + prop.declarations?.some((declaration) => !declaration.fileName.includes("node_modules")) || + prop.name === "children" || + !prop.parent || + !prop.parent.fileName.includes("node_modules"), }); // `parser.parse` rebuilds TypeScript program state and is expensive; several pages diff --git a/apps/docs/src/pages/components/context-menu.mdx b/apps/docs/src/pages/components/context-menu.mdx index 06ec2c33..d1866a33 100644 --- a/apps/docs/src/pages/components/context-menu.mdx +++ b/apps/docs/src/pages/components/context-menu.mdx @@ -17,6 +17,8 @@ import selectionRowsSource from "~/demos/context-menu/selection-rows.tsx?raw"; ## Basic +Rows take `variant` for their look — `neutral` (the default) and `danger` for a destructive action. + @@ -39,6 +41,10 @@ Nest a `ContextMenuSubmenu` to reveal more destinations beside the parent menu. ## Selection rows +`ContextMenuCheckboxItem` toggles independently; `ContextMenuRadioItem` rows wrapped in a +`ContextMenuRadioGroup` share one `value`. Both pass `closeOnClick={false}` so the menu stays open +while you adjust the options. + @@ -55,11 +61,39 @@ import { } from "@makeplane/propel/components/context-menu"; ``` -## Props +## API Reference diff --git a/apps/docs/src/pages/components/menu.mdx b/apps/docs/src/pages/components/menu.mdx index 61645468..d0931f14 100644 --- a/apps/docs/src/pages/components/menu.mdx +++ b/apps/docs/src/pages/components/menu.mdx @@ -19,6 +19,9 @@ import submenuSource from "~/demos/menu/submenu.tsx?raw"; ## Basic +Rows take `variant` for their look — `neutral` (the default), `accent` for the primary action, and +`danger` for a destructive one. + @@ -67,12 +70,26 @@ import { } from "@makeplane/propel/components/menu"; ``` -## Props +## API Reference diff --git a/packages/propel/.storybook/main.ts b/packages/propel/.storybook/main.ts index 4ee73068..724c8ad1 100644 --- a/packages/propel/.storybook/main.ts +++ b/packages/propel/.storybook/main.ts @@ -42,8 +42,17 @@ const config: StorybookConfig = { // (e.g. the atomic slot parts that take a bare icon / svg / label). Keeping it here // surfaces those opt-in, TSDoc'd `children` slots instead of an empty "couldn't be // auto-generated" args table, without adding a `children` row to every component. + // + // The `declarations` test comes first for a related reason: a prop propel declares itself + // but which SHADOWS a native attribute (`placeholder`, `value`, `type`) gets `parent` set + // to the @types/react declaration, so the parent-only test dropped it. `declarations` lists + // every declaration site, so such a prop survives when any of them is outside node_modules. + // Kept in sync with apps/docs/src/lib/props-schema.ts, which parses the same sources. propFilter: (prop) => - prop.name === "children" || !prop.parent || !prop.parent.fileName.includes("node_modules"), + prop.declarations?.some((declaration) => !declaration.fileName.includes("node_modules")) || + prop.name === "children" || + !prop.parent || + !prop.parent.fileName.includes("node_modules"), }, }, // Storybook runs its own Vite build; add the Tailwind v4 plugin so propel's diff --git a/packages/propel/AGENTS.md b/packages/propel/AGENTS.md index bb56aeb4..6f2c1914 100644 --- a/packages/propel/AGENTS.md +++ b/packages/propel/AGENTS.md @@ -34,8 +34,8 @@ from tiers below it, never above. `internal/` is shared implementation usable by - **Don't `export *` from `./variants`** — it holds cvas and the `VariantProps` cva-props bundle, which stay **private**: a part imports `VariantProps` for its own `Props` (rule 10) but **never re-exports it**, and a cva is **never** re-exported (not by the part, not by the index, - not across tiers). Only the **per-axis** types (`Magnitude`, `Tone`, …) are public — - re-exported by the part's component file (`export type { Magnitude } from "./variants"`), + not across tiers). Only the **per-axis** types (`Variant`, `Size`, …) are public — + re-exported by the part's component file (`export type { Variant } from "./variants"`), which the index then stars. Renames (`export { X as Y }`) also stay explicit. ## Hard rules @@ -60,7 +60,7 @@ primitive it merely _controls_ (`Collapsible.Trigger` under a list section, a me opens a `Menu`), or a consumer-supplied control (`Clear`/`Trigger` as node props): ```tsx -} placeholder={…} /> +} placeholder={…} /> }>… }>… @@ -89,7 +89,7 @@ carries no `className`, so its size comes from the `elements` cva). `lucide-reac 2b. **`elements` stories are pure UI-configuration showcases; behavior stories live in `components`.** An `elements` story renders the styled parts DIRECTLY — every visual axis -(`magnitude`/`tone`/…) and every visual state, the states pinned statically via the `data-*`/aria +(`variant`/`size`/…) and every visual state, the states pinned statically via the `data-*`/aria attributes Base UI would set (`data-pressed=""`, `data-highlighted=""`, `data-panel-open=""`, `data-invalid=""`, `aria-current="page"`, …). It imports **nothing** from `@base-ui/react` and never from `components`, and it carries **no behavior `play` tests** (a hidden CSS canary asserting @@ -145,24 +145,24 @@ live in `components`, keeping their flattened Base-UI name (`Autocomplete`, not An identical styled element shared across families is a single `internal/` primitive, so the family does not re-export it (rule 4a). -6b. **Custom prop names never reuse a native HTML/CSS attribute name.** `width`, `height`, `size`, -`type`, `color` are off-limits as style/layout props — an `elements` part is render-capable, so the name -would collide with the element's own attribute (and confuse readers). Use the controlled axis -vocabulary instead: `variant`, `tone`, `magnitude`, `emphasis`, `surface`, `density`, `elevation`, -`orientation`, `sizing`. (Full-width-vs-hug is `sizing: "hug" | "fill"` — mirroring Figma's resize — -not `width`.) Genuine native attributes (`type`, `disabled`, `href`, `aria-*`) pass through untouched. - -6c. **`variant` is a smell — name the real axis, and split when values are different elements.** -A prop named `variant` is almost always too vague: the values express a specific concept -(`prominence`, `appearance`, `mode`, `layout`, `placement`) — name _that_. If the values would -render a **different element or semantics** (e.g. `