-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Blocks: Share window listeners across instances (block props, rich text, ...) #78310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
3c24501
Rich text: Share window listeners across RichText instances
ellatrix 0c8e189
Rich text: Extend shared listeners to block-editor's enter and paste …
ellatrix 9d6b776
Rich text: Share the keydown listener for full-content delete
ellatrix 9de866d
Rich text: Share the keydown listener for format boundaries
ellatrix f7a6880
Rich text: Use capture phase for format-boundaries shared listener
ellatrix b508cdc
Rich text: Share the click and focusin listeners for object selection
ellatrix 769437c
Rich text: Share the pointerdown and keydown listeners for selection-…
ellatrix d4c1ae8
Rich text: Share the input/composition/focus listeners
ellatrix a66ea64
Rich text: Use capture phase for input and compositionend shared list…
ellatrix 94939d6
Rich text: Bail in onFocus when contentEditable is false
ellatrix 8d6cd58
Block editor: Share the focusin listener for per-block focus handling
ellatrix 4b784e1
Block editor: Share the mouseover and mouseout listeners for hover
ellatrix 9ef966a
Compose: Hoist subscribeSharedListener from rich-text
ellatrix 277c285
Compose: Add TypeScript types to subscribeSharedListener
ellatrix b6c77af
Autocomplete: Share the keydown listener across instances
ellatrix 7a4a92a
Autocomplete: Use Event type for shared listener callback
ellatrix eb2415e
Block library: Share per-block keydown listeners for use-enter/use-space
ellatrix 6e208e4
Components: Add CHANGELOG entry for Autocomplete listener sharing
ellatrix 51a5355
Use === guards in keydown/input shared listeners
ellatrix e3681e8
Compose: subscribeSharedListener accepts Element for ancestry dispatch
ellatrix ec3ae61
Compose: Fix subscribeSharedListener cross-realm Document/Window check
ellatrix 9cda3ba
Compose: Make subscribeSharedListener private
ellatrix b11e62f
Private APIs: Allow @wordpress/compose to opt in
ellatrix 55e94fa
Compose: Reference @wordpress/private-apis in tsconfig
ellatrix 7c84727
Compose: Update lockfile for @wordpress/private-apis dependency
ellatrix 81c691d
Compose: Weakly hold element subscribers in subscribeSharedListener
ellatrix aa309f0
Compose: Rename subscribeSharedListener → subscribeDelegatedListener
ellatrix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't be afraid to make it public immediately. It's a solid API and there are no doubts about its usefulness.
One little suggestion: instead of the
captureparam, supportoptions = { capture }object. Or both, the goal is symmetry withaddEventListener.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing I'm not sure of is the naming 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about
subscribeDocumentListener, but that doesn't work very well forwindow🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addSharedListener?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be "add" because it doesn't behave like
addEventListener. It returns an unsubscribe callback, there is no "remove" function likeremoveEventListener. So, the "subscribe" part is right 🙂 It's about the adjective: "shared listener", "document listener"?I asked Cursor (inside React repo context, where this pattern is used for the entire event system) and it says firmly it should be
subscribeDelegatedListener, insisting that "event delegation" is a standard established name for what we're doing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like subscribeDelegatedListener
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed, but I'll leave it private for now. It's always easy to make public.