Skip to content
Open
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
33 changes: 13 additions & 20 deletions apps/storybook/stories/comments-dropdown.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type { Meta, StoryObj } from "storybook-solidjs-vite";
import { expect, waitFor } from "storybook/test";
import { CommentsDropdown } from "react-grab/src/components/comments-dropdown.js";
import type { DropdownAnchor } from "react-grab/src/types.js";
import { assertMounted } from "./assertions.js";
import type { CommentPreset } from "./fixtures.js";
import { COMMENT_PRESET_KEYS, getItemPresets } from "./fixtures.js";
import { Canvas } from "./target-box.js";
import { noop } from "./noop.js";

interface CommentsDropdownSceneProps {
Expand All @@ -21,25 +20,19 @@ const DROPDOWN_ANCHOR: DropdownAnchor = {
const meta: Meta<CommentsDropdownSceneProps> = {
title: "Components/CommentsDropdown",
render: (args) => (
<Canvas>
<CommentsDropdown
position={DROPDOWN_ANCHOR}
items={getItemPresets()[args.preset]}
onSelectItem={noop}
onItemHover={noop}
onCopyAll={noop}
onCopyAllHover={noop}
onClearAll={noop}
onDismiss={noop}
onDropdownHover={noop}
/>
</Canvas>
<CommentsDropdown
position={DROPDOWN_ANCHOR}
items={getItemPresets()[args.preset]}
onSelectItem={noop}
onItemHover={noop}
onCopyAll={noop}
onCopyAllHover={noop}
onClearAll={noop}
onDismiss={noop}
onDropdownHover={noop}
/>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Play functions in story files don't await promise-returning assertion functions, causing tests to complete before assertions run

Fix on Vercel

),
play: async ({ canvasElement }) => {
await waitFor(() => {
expect(canvasElement.querySelector("[data-react-grab-comments-dropdown]")).not.toBeNull();
});
},
play: ({ canvasElement }) => assertMounted(canvasElement, "[data-react-grab-comments-dropdown]"),
args: { preset: "multiple" },
argTypes: {
preset: { control: "select", options: COMMENT_PRESET_KEYS },
Expand Down
14 changes: 5 additions & 9 deletions apps/storybook/stories/context-menu.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { Meta, StoryObj } from "storybook-solidjs-vite";
import { expect, waitFor } from "storybook/test";
import { ContextMenu } from "react-grab/src/components/context-menu.js";
import type { Position } from "react-grab/src/types.js";
import { assertMounted } from "./assertions.js";
import { DEMO_BOUNDS } from "./demo-bounds.js";
import { createMenuActions } from "./fixtures.js";
import { Canvas, TargetBox } from "./target-box.js";
import { TargetBox } from "./target-box.js";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Canvas component is now unused dead code

Low Severity

This PR removes every import of Canvas from all story files, but the Canvas component (and its CanvasProps interface) remain exported in target-box.tsx. Since no file in the codebase imports Canvas anymore, it's dead code that can be cleaned up along with the now-unused JSX type import in that file.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 60655c0. Configure here.

import { noop } from "./noop.js";

interface ContextMenuSceneProps {
Expand All @@ -21,7 +21,7 @@ const MENU_POSITION: Position = {
const meta: Meta<ContextMenuSceneProps> = {
title: "Components/ContextMenu",
render: (args) => (
<Canvas>
<>
<TargetBox />
<ContextMenu
position={MENU_POSITION}
Expand All @@ -33,13 +33,9 @@ const meta: Meta<ContextMenuSceneProps> = {
onDismiss={noop}
onHide={noop}
/>
</Canvas>
</>
),
play: async ({ canvasElement }) => {
await waitFor(() => {
expect(canvasElement.querySelector("[data-react-grab-context-menu]")).not.toBeNull();
});
},
play: ({ canvasElement }) => assertMounted(canvasElement, "[data-react-grab-context-menu]"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
play: ({ canvasElement }) => assertMounted(canvasElement, "[data-react-grab-context-menu]"),
play: async ({ canvasElement }) => {
await assertMounted(canvasElement, "[data-react-grab-context-menu]");
},

The play function doesn't await the assertMounted promise, causing the test to complete before the assertion runs

Fix on Vercel

args: { tagName: "button", componentName: "Button", hasFilePath: false },
};

Expand Down
239 changes: 23 additions & 216 deletions apps/storybook/stories/renderer.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { createEffect, createSignal, on, onCleanup, onMount } from "solid-js";
import type { Meta, StoryContext, StoryObj } from "storybook-solidjs-vite";
import { expect, waitFor } from "storybook/test";
import { ReactGrabRenderer } from "react-grab/src/components/renderer.js";
import type { OverlayBounds } from "react-grab/src/types.js";
import { assertMounted, assertNotMounted } from "./assertions.js";
import { COMMENT_PRESET_KEYS, createMenuActions, getItemPresets } from "./fixtures.js";
import type { CommentPreset } from "./fixtures.js";
import { noop } from "./noop.js";
import { SampleDashboard } from "./sample-dashboard.js";

const ELEMENT_KEYS = [
"none",
Expand All @@ -23,10 +24,13 @@ const ELEMENT_KEYS = [

type ElementKey = (typeof ELEMENT_KEYS)[number];

interface ElementMeta {
tagName: string;
componentName: string;
}
const isElementKey = (value: string | undefined): value is ElementKey => {
if (value === undefined) return false;
for (const candidate of ELEMENT_KEYS) {
if (candidate === value) return true;
}
return false;
};

const measureElement = (element: HTMLElement | undefined): OverlayBounds | undefined => {
if (!element) return undefined;
Expand Down Expand Up @@ -59,14 +63,6 @@ const Scene = (props: SceneProps) => {
const elementRefs: Partial<Record<ElementKey, HTMLElement>> = {};
const [selectionBounds, setSelectionBounds] = createSignal<OverlayBounds | undefined>(undefined);

const isElementKey = (value: string | undefined): value is ElementKey => {
if (value === undefined) return false;
for (const candidate of ELEMENT_KEYS) {
if (candidate === value) return true;
}
return false;
};

const captureRef = (element: HTMLElement | undefined): void => {
if (!element) return;
const key = element.dataset.storyId;
Expand Down Expand Up @@ -103,7 +99,7 @@ const Scene = (props: SceneProps) => {

createEffect(on(() => props.selectedElement, scheduleRecompute, { defer: true }));

const elementMeta = (): ElementMeta => {
const elementMeta = () => {
const element = elementRefs[props.selectedElement];
if (!element) return { tagName: "", componentName: "" };
return {
Expand All @@ -129,191 +125,8 @@ const Scene = (props: SceneProps) => {
const commentItems = () => getItemPresets()[props.commentPreset] ?? [];

return (
<div
style={{
"min-height": "100vh",
background: "#fafafa",
"font-family": "-apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif",
color: "#1a1a1a",
}}
>
<header
ref={captureRef}
data-story-id="header"
data-component="AppHeader"
style={{
display: "flex",
"align-items": "center",
"justify-content": "space-between",
padding: "16px 32px",
background: "#fff",
"border-bottom": "1px solid #e5e5e5",
}}
>
<h1
ref={captureRef}
data-story-id="logo"
data-component="Logo"
style={{
"font-size": "18px",
"font-weight": 700,
margin: 0,
"letter-spacing": "-0.02em",
}}
>
Acme Dashboard
</h1>
<nav style={{ display: "flex", gap: "24px", "font-size": "14px" }}>
<a
ref={captureRef}
data-story-id="nav-home"
data-component="NavLink"
href="#"
style={{ color: "#1a1a1a", "text-decoration": "none", "font-weight": 500 }}
>
Home
</a>
<a
ref={captureRef}
data-story-id="nav-about"
data-component="NavLink"
href="#"
style={{ color: "#666", "text-decoration": "none" }}
>
About
</a>
</nav>
</header>

<main
style={{
display: "grid",
"grid-template-columns": "1fr 1fr",
gap: "24px",
padding: "32px",
"max-width": "880px",
margin: "0 auto",
}}
>
<div
ref={captureRef}
data-story-id="card-welcome"
data-component="WelcomeCard"
style={{
background: "#fff",
"border-radius": "12px",
padding: "24px",
"box-shadow": "0 1px 3px rgba(0,0,0,0.08)",
border: "1px solid #e5e5e5",
}}
>
<h2 style={{ "font-size": "16px", "font-weight": 600, margin: "0 0 8px" }}>Welcome</h2>
<p
style={{
"font-size": "14px",
color: "#666",
margin: "0 0 20px",
"line-height": 1.5,
}}
>
This is a sample dashboard. Select any element using the controls below to see React
Grab's overlay.
</p>
<button
ref={captureRef}
data-story-id="btn-start"
data-component="Button"
style={{
padding: "8px 16px",
background: "#1a1a1a",
color: "#fff",
border: "none",
"border-radius": "8px",
"font-size": "14px",
"font-weight": 500,
cursor: "pointer",
}}
>
Get Started
</button>
</div>

<div
ref={captureRef}
data-story-id="card-settings"
data-component="SettingsCard"
style={{
background: "#fff",
"border-radius": "12px",
padding: "24px",
"box-shadow": "0 1px 3px rgba(0,0,0,0.08)",
border: "1px solid #e5e5e5",
}}
>
<h2 style={{ "font-size": "16px", "font-weight": 600, margin: "0 0 8px" }}>Settings</h2>
<label
style={{
"font-size": "13px",
color: "#666",
display: "block",
"margin-bottom": "6px",
}}
>
Display name
</label>
<input
ref={captureRef}
data-story-id="input"
data-component="TextField"
type="text"
placeholder="Your name"
style={{
width: "100%",
padding: "8px 12px",
border: "1px solid #d4d4d4",
"border-radius": "8px",
"font-size": "14px",
"margin-bottom": "16px",
"box-sizing": "border-box",
outline: "none",
}}
/>
<button
ref={captureRef}
data-story-id="btn-save"
data-component="Button"
style={{
padding: "8px 16px",
background: "#fff",
color: "#1a1a1a",
border: "1px solid #d4d4d4",
"border-radius": "8px",
"font-size": "14px",
"font-weight": 500,
cursor: "pointer",
}}
>
Save Changes
</button>
</div>
</main>

<footer
ref={captureRef}
data-story-id="footer"
data-component="Footer"
style={{
padding: "24px 32px",
"text-align": "center",
"font-size": "13px",
color: "#999",
"border-top": "1px solid #e5e5e5",
"margin-top": "48px",
}}
>
© 2025 Acme Inc. All rights reserved.
</footer>

<>
<SampleDashboard captureRef={captureRef} />
<ReactGrabRenderer
selectionVisible={hasSelection()}
selectionBounds={selectionBounds()}
Expand Down Expand Up @@ -351,27 +164,23 @@ const Scene = (props: SceneProps) => {
onContextMenuDismiss={noop}
onContextMenuHide={noop}
/>
</div>
</>
);
};

const meta: Meta<SceneProps> = {
title: "Playground",
render: (args) => <Scene {...args} />,
play: async ({ canvasElement, args }) => {
const selectionLabel = () => canvasElement.querySelector("[data-react-grab-selection-label]");
const toolbar = () => canvasElement.querySelector("[data-react-grab-toolbar]");

await waitFor(() => {
if (args.selectedElement === "none") {
expect(selectionLabel()).toBeNull();
} else {
expect(selectionLabel()).not.toBeNull();
}
if (args.showToolbar) {
expect(toolbar()).not.toBeNull();
}
});
const selector = "[data-react-grab-selection-label]";
if (args.selectedElement === "none") {
await assertNotMounted(canvasElement, selector);
} else {
await assertMounted(canvasElement, selector);
}
if (args.showToolbar) {
await assertMounted(canvasElement, "[data-react-grab-toolbar]");
}
},
args: {
selectedElement: "btn-start",
Expand Down Expand Up @@ -416,9 +225,7 @@ export const ContextMenu: Story = {
play: async (context: StoryContext<SceneProps>) => {
if (!meta.play) throw new Error("meta.play is required for shared assertions");
await meta.play(context);
await waitFor(() => {
expect(context.canvasElement.querySelector("[data-react-grab-context-menu]")).not.toBeNull();
});
await assertMounted(context.canvasElement, "[data-react-grab-context-menu]");
},
};

Expand Down
Loading
Loading