Skip to content

[react-dom] Fix non-popover <dialog> ignoring inline toggle events#693

Open
everettbu wants to merge 1 commit intomainfrom
fix-dialog-toggle-listeners
Open

[react-dom] Fix non-popover <dialog> ignoring inline toggle events#693
everettbu wants to merge 1 commit intomainfrom
fix-dialog-toggle-listeners

Conversation

@everettbu
Copy link
Copy Markdown

Mirror of facebook/react#36015
Original author: nad767


Fixes a bug with native <dialog> elements ignoring event handlers provided in onBeforeToggle and onToggle props, unless they also happen to have the popover attribute.

Per MDN, the two abovementioned events should fire for <dialog> elements regardless of whether they are popovers or not,1 and indeed, calling someNonPopoverDialogRef.current.addEventListener('beforetoggle'|'toggle', someCallback) does in fact work (tested in Chrome 146).

Footnotes

  1. See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/beforetoggle_event and https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/toggle_event.

Fixes a bug with native `<dialog>` elements ignoring event handlers provided in `onBeforeToggle` and `onToggle` props, unless they also happen to have the `popover` attribute.

Per MDN, the two abovementioned events should fire for `<dialog>` elements regardless of whether they are `popover`s or not,[^1] and indeed, calling `someNonPopoverDialogRef.current.addEventListener('beforetoggle'|'toggle', someCallback)` does in fact work (tested in Chrome 146).

[^1]: See https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/beforetoggle_event and https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/toggle_event.
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR fixes a bug where onBeforeToggle and onToggle event handlers were silently ignored on non-popover <dialog> elements that were server-side rendered and hydrated. The fix adds two missing listenToNonDelegatedEvent calls in hydrateProperties, bringing it in line with createDOMElement which already correctly registered these listeners.

  • The root cause was a gap between createDOMElement (client-only render path, lines ~1359–1364) and hydrateProperties (SSR hydration path, lines ~3113–3120): beforetoggle and toggle were present in the former but missing from the latter.
  • For <dialog popover> elements, these events still fired during hydration because a separate props.popover != null check (line ~3240) registered them — which is why the bug only surfaced for non-popover dialogs.
  • The two-line fix is minimal, surgical, and correctly mirrors the existing createDOMElement behavior.

Confidence Score: 5/5

  • This PR is safe to merge — it's a minimal, targeted bug fix with no risk of regression.
  • The change is two lines, exactly mirrors what createDOMElement already does for dialog, and closes a clear asymmetry between the client-render and hydration paths. No existing behavior is altered; only previously broken event wiring is restored.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/react-dom-bindings/src/client/ReactDOMComponent.js Adds beforetoggle and toggle non-delegated event listeners to dialog elements in hydrateProperties, mirroring the existing listeners already present in createDOMElement. This corrects an oversight where SSR-hydrated non-popover <dialog> elements would silently ignore onBeforeToggle/onToggle props.

Last reviewed commit: a8d0066

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants