Image Upload Components — Learnings

Knowledge discovered during the implementation of 19 image upload components in the ux-prototype canary library, useful for future tasks.

L-01: Bake interactions into components, don’t externalize them

The initial implementation (Runs 1-3) externalized dialog-opening and state management into stories and consumers. This led to TODO stubs, duplicative hover overlays, and broken user-facing actions (ImageFormField edit/inspect buttons that did nothing).

The post-run refinement phase baked interactions directly into components: ImageDisplay gained onImageChange + config props that internally open ImageUploadDialog on double-click/Enter. ImageInspectorOverlay gained an uncontrolled mode. ImageComparisonLayout gained baked-in action buttons.

Principle: Components should be self-contained interaction units. If a consumer must wire a dialog, manage open/close state, or compose multiple components to achieve a basic flow, the abstraction boundary is wrong. Externalize only the result (callbacks like onImageChange), not the mechanism (dialog state, trigger handlers).

L-02: AG Grid cell editors require imperative patterns that diverge from React composition

ImageCellEditor could not reuse ImageDisplay’s baked-in dialog flow because AG Grid’s cell editor lifecycle requires:

isPopup() returning true to prevent focus-loss from stopping editing
getValue() for imperative value extraction
stopEditing() for programmatic editing termination
Immediate dialog open on mount (not on user double-click)

This is a justified duplication of the dialog wiring pattern. Document intentional divergences from composition patterns when framework constraints require them.

L-03: `pointer-events-none` + `pointer-events-auto` for overlay affordances

When hover action icons (eye, trash) must overlay an interactive element (like ImageDisplay’s button), use pointer-events-none on the overlay container and pointer-events-auto only on the individual icon buttons. This allows clicks on the image area to pass through to the underlying button while still allowing clicks on the icons.

L-04: Mock imports in production code are a silent correctness hazard

During the mock-data-first implementation approach, ImageUploadDialog imported mockUpload and mockReachabilityCheck from __mocks__/. This compiled and ran correctly in Storybook — there was no lint error, no type error, and no visible bug. The hazard is invisible until someone ships the component to a real application.

Mitigation: Use dependency injection (props or config) for any function that will have a real implementation. Never import from __mocks__/ in production component files. ESLint no-restricted-imports could enforce this statically.

L-05: `stopEditingWhenCellsLoseFocus` must be `false` for popup editors

AG Grid’s default stopEditingWhenCellsLoseFocus: true causes popup cell editors (like ImageCellEditor with isPopup()) to immediately stop editing when the popup receives focus, because focus leaves the grid cell. Setting stopEditingWhenCellsLoseFocus: false on the DataGrid allows popup editors to function correctly.

L-06: `createWorkflowStories` generalizes the use-case framework

The original use-case story framework was tightly coupled to specific story shapes. Extracting createWorkflowStories as a generic factory that accepts renderScene (for stepwise) and renderLive (for interactive) callbacks enables any multi-step workflow — dialog flows, component interactions, form wizards — to produce Interactive/Stepwise/Automated story variants without duplicating the framework infrastructure.

L-07: Storybook sidebar ordering requires explicit enforcement

With 19 new components across 15 story files, sidebar ordering became inconsistent. The convention “Playground story always last” required a custom enforce-story-order.js script (tools/enforce-story-order.js) that validates story export order across all files. Run this as part of the lint pass for projects with many story files.

L-08: ShadCN primitives need selector fixes after vendoring

Vendored ShadCN primitives may use incorrect Radix data attribute selectors. Two examples found:

Tabs: data-horizontal:flex-col should be data-[orientation=horizontal]:flex-col (Radix sets data-orientation="horizontal", not data-horizontal).
Slider: track height was conditional on orientation but should be unconditional for the common horizontal-only case.

Always verify Radix data attribute selector syntax against the actual DOM output when vendoring ShadCN primitives.