Skip to content

fix(ui): stop Button throwing away a click when something re-renders - #52

Merged
dementive merged 1 commit into
mainfrom
paul/ui-button-clicks
Aug 27, 2026
Merged

fix(ui): stop Button throwing away a click when something re-renders#52
dementive merged 1 commit into
mainfrom
paul/ui-button-clicks

Conversation

@pclauss123

Copy link
Copy Markdown
Contributor

Summary

Button throws away a click when anything re-renders mid-press. This fixes it.

A click is only dispatched when mousedown and mouseup land on the same
element
. The button's inner surface was a component declared insideButton:

constContentWrapper=({ children }: {children: ReactNode})=>(// ← new type every render

A component declared during render is a new component type on every render, so React
unmounts the content subtree and mounts a fresh one each time. Any render occurring
between the two halves of a press replaces the element the pointer is over, and the
click never happens. A hover handler on an ancestor is enough to cause that render,
which is why it presented as intermittent.

Nothing said anything was wrong. The button stayed in the tree, matched by role and
name, and reported enabled throughout. It cost two long debugging sessions in the part
viewer before the cause was found, and it is currently eating the Delete button in
the DFM app — one e2e failure there traces to exactly this.

The surface is now an element built by a plain function rather than a component
declared during render, so its identity survives.

On the test

Asserted as node identity across a re-render, not by firing a click. That is
deliberate: fireEvent.click dispatches the event straight at the element, and jsdom
does not build a click out of mousedown and mouseup — so a click-based test passes
whether or not the surface was replaced. I wrote that version first and confirmed it
passes against the broken component, which is worth knowing if anyone reaches for it
again.

Verified the test fails against the current main component and passes against the
fix:

git checkout origin/main -- src/button/button.tsx && npx vitest run
AssertionError: expected <div …(1)></div> to be <div …(1)></div> // Object.is equality
Tests 1 failed | 4 passed (5)

Public package release

  • This PR changes no public package source.
  • I ran pnpm changeset and committed the generated Changeset.
  • This source change intentionally needs no release; I applied the
    no-release-needed label and explained why below.

patch — a bug fix, no API change.

Validation

  • I ran the relevant checks and tests.
pnpm --filter @toolpath/ui test 2 files, 5 tests, green
pnpm --filter @toolpath/ui check-types clean
pnpm --filter @toolpath/ui build clean, dist/ emitted
node scripts/check-release-intent.mjs origin/main
Release intent recorded for: @toolpath/ui

Independent of #51 — different package, no overlap. Either can land first.

A click is only dispatched when mousedown and mouseup land on the same
element. Button's inner surface was a component declared inside Button, which
makes it a new component type on every render — so React unmounted the
content subtree and mounted a fresh one, and any render occurring between the
two halves of a press replaced the element the pointer was over. The click
never happened.
A hover handler on an ancestor is enough to cause that render, which is why it
presented as intermittent. Nothing said anything was wrong: the button stayed
in the tree, matched by role and name, and reported enabled throughout. It
cost two long debugging sessions in the part viewer before the cause was
found, and it is currently eating the Delete button in the DFM app.
The surface is now an element built by a plain function rather than a
component declared during render, so its identity survives.
Asserted as node identity rather than by firing a click. fireEvent.click
dispatches straight at the element and jsdom does not build a click out of
mousedown and mouseup, so a click-based test passes whether the surface was
replaced or not — the identity is what the browser actually cares about.
@dementive
dementive merged commit 00d0333 into mainAug 27, 2026
2 checks passed
Sign up for freeto 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.

3 participants

@pclauss123@BradEstey@dementive