Add shared editor content token registry - #1372
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5288e9c436
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| surface: 'block-editor-rich-text', | ||
| postId: editor.getCurrentPostId() as number, | ||
| postType: editor.getCurrentPostType() as string, | ||
| blockName: blockEditor.getSelectedBlock()?.name, | ||
| } as ContentTokenContext; |
There was a problem hiding this comment.
When a token declares conditions.attributes, this RichText surface never supplies attributeName in the context, so isContentTokenAvailable() always rejects it via listAllows. Consequently every attribute-constrained token is permanently absent from the picker, including on the intended attribute; derive and include the active RichText attribute before filtering.
Useful? React with 👍 / 👎.
| while ( start > 0 && hasFormat( formats[ start - 1 ], type ) ) { | ||
| start--; | ||
| } | ||
| while ( end < formats.length && hasFormat( formats[ end ], type ) ) { | ||
| end++; |
There was a problem hiding this comment.
When two editable tokens are adjacent, their per-character formats have the same type but different valueId attributes. These loops therefore merge both tokens into one active range, so replacing either token deletes and replaces its neighbor as well; range expansion must compare the active format's attributes or token ID in addition to its type.
Useful? React with 👍 / 👎.
| export const registerContentToken = ( token: ContentTokenDefinition ): void => { | ||
| contentTokens.register( token ); | ||
| }; |
There was a problem hiding this comment.
When a provider re-registers an existing token ID under a different group, this delegates to createRegistry, whose existing-item check requires both the ID and group to match; the new registration is therefore appended instead of replacing the old one. The picker then exposes two definitions for the same canonical saved ID, contrary to this API's replace contract, so replacement needs to be keyed solely by the stable token ID.
Useful? React with 👍 / 👎.
| registerFormatType( EDITABLE_FORMAT, { | ||
| title: __( 'Editable dynamic value', 'popup-maker' ), | ||
| tagName: 'data', | ||
| className: 'pum-content-token-editable', | ||
| attributes: { valueId: 'value' }, |
There was a problem hiding this comment.
When an author lacks unfiltered_html—including ordinary Editor/Author accounts and non-super-admins on multisite—WordPress post KSES does not allow the HTML <data> element, so saving strips this wrapper and its value attribute while leaving only the preview text. The canonical token ID is then permanently lost and server-side resolution cannot recognize the token; use an allowed element such as span with an allowed data-* attribute, or explicitly permit and preserve this element.
Useful? React with 👍 / 👎.
| const tokens = contentTokens.useItems(); | ||
| const groups = contentTokenGroups.useItems(); |
There was a problem hiding this comment.
When a provider registers its first token or group after this edit component has mounted, createRegistry.register() appends to its items array in place and useItems() returns that same array as the useSyncExternalStore snapshot. React compares snapshots by identity, sees no change after emitChange(), and leaves the picker stale until some unrelated render occurs; the registry must publish a new snapshot for additions and group-driven sorting.
Useful? React with 👍 / 👎.
1 participant
Adds the reusable editor content-token projection used by Pro Lead Magnets and future Core/Pro/Pro+ features.
Pro dependency: PopupMaker/Pro#183
Validation: