Skip to content

fix(useOnClickOutside): don't add document listeners while disabled - #690

Open
blvdmitry wants to merge 2 commits into
mainfrom
fix/use-on-click-outside-disabled-listeners
Open

blvdmitry wants to merge 2 commits into
mainfrom
fix/use-on-click-outside-disabled-listeners

Conversation

@blvdmitry

@blvdmitry blvdmitry commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Issue. useOnClickOutside added its mousedown / touchstart / keydown document listeners whatever disabled was, and checked disabled only in the click handler. The comment gave the reason: "some components can enable the hook after it was clicked". The case it protects is a Flyout with triggerType="focus", which Autocomplete uses. The mousedown on the trigger focuses it, the focus opens the flyout, and that enables the hook. The hook then has to know the press started inside, or the click that follows would close the flyout straight away.

The cost is that every disabled instance still adds 3 document listeners, and each one runs on every mousedown and keypress on the page. This was found while profiling a 5,000-row menu in Silo (reshaped-ui/silo#104). Every row's editable title calls the hook with disabled: !editing. Adding those listeners took about 10% of the menu's mount time, and every click then ran thousands of handlers that did nothing.

Fix.

  • The mousedown effect now returns early when the hook is disabled, so a disabled hook adds no document listeners.
  • When the hook becomes enabled, isMouseDownInsideRef is reset to null, meaning no mousedown has been seen since it was enabled. Until the next mousedown, the click handler checks the click's own target against the refs instead, so a click that started inside before the hook was enabled still counts as inside.
  • After the first mousedown, the behaviour is the same as before. There is no module-level state.
  • The public signature and enabled behaviour are unchanged.

Tests (Storybook play tests in useOnClickOutside.stories.tsx):

  • test: disabled doesn't add listeners: mounts 10 disabled hooks and checks that none of the hook's event types get a document listener. It fails on the old hook.
  • test: enabled during click inside: the hook is enabled on an input's onFocus. Clicking the input must not count as an outside click, and a later click on the body must call the handler. It fails without the click-target fallback.

Also adds a patch changeset.

Related Issue

Found while profiling reshaped-ui/silo#104.

Screenshots / Recordings

N/A, no UI changes.

Notes for Reviewers

  • The click-target fallback applies only to the first click after the hook is enabled, and only when there was no mousedown in between. Two cases differ from the old mousedown check:
    • The click target is removed from the DOM before the click reaches document. The click then counts as outside.
    • A press starts inside, before the hook is enabled, and is released outside. The click target is then outside, so the click counts as outside.
  • Checked locally: a temporary story that clicks into an Autocomplete input. The list stays open with this change and closes when the hook only returns early. The existing Autocomplete tests don't click into the input.
  • Checks run locally: pnpm lint:js, oxfmt --check on the changed files, the tsc typechecks, pnpm test:unit and the full browser suite all pass. The only browser failure was Image onLoad, which loads an image from unsplash.com and could not reach it from the sandbox.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SM7EVWJmvGVrdoX9HATtZR

Disabled hooks used to keep their mousedown/touchstart/keydown listeners
on the document so that components enabling the hook mid-click (e.g. a
Flyout with focus trigger opening on mousedown) would still know the
mousedown happened inside. A single shared listener now records the last
mousedown target, and the hook checks it when it gets enabled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SM7EVWJmvGVrdoX9HATtZR
@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Library / JS 51.87 KB (+0.09% 🔺) 1.1 s (+0.09% 🔺) 1.3 s (-3.64% 🔽) 2.3 s
Library / CSS 23.88 KB (0%) 478 ms (0%) 0 ms (+100% 🔺) 478 ms
Theming / JS 7.78 KB (0%) 156 ms (0%) 89 ms (-15.24% 🔽) 244 ms
Theming with a default theme definition / JS 8.69 KB (0%) 174 ms (0%) 153 ms (+40.34% 🔺) 327 ms

…ed mousedown listener

Drops the module-level mousedown tracking. When the hook gets enabled it
doesn't know where the last mousedown happened yet, so until the next
mousedown it checks the click target against the refs instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SM7EVWJmvGVrdoX9HATtZR

This branch has not been deployed

No deployments
Sign up for free to 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.

2 participants