Repository navigation
fix(admin): make Buttons blocks editable in the rich text editor - #4595
Conversation
github-actions
Bot
added
review/needs-review
🦋 Changeset detectedLatest commit: 813938b The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Registering a built-in buttons definition through the existing Block Kit plugin-block path is the right approach. It lets imported groups open a real form and lets /buttons insert a group. It also defers to a plugin's own buttons definition, which fits the editor's architecture. The main issue is that _key values are reused by array position, so deleting or reordering buttons moves keys onto different buttons. This contradicts the claim that existing item keys are kept. Both reviewers raised it. Anthropic rated it a non-blocking suggestion and said it is harmless for Buttons.astro. OpenAI rated it needs_fixing because consumers that track keyed items could be misled. I kept the higher severity because the stated reasons (identity drift, no tests) support it. Separately, OpenAI found that the new test omits the .js import extensions required by the repo's ESM convention. Anthropic did not mention this. Neither reviewer ran tests.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
| return { | ||
| ...rest, | ||
| _type: "button", | ||
| _key: previousKeys[index] || generateKey(), |
There was a problem hiding this comment.
[needs fixing] This assigns an existing key by array position (previousKeys[index]), not by button identity. BlockKitRepeater strips _key whenever it emits after edits, deletions, and drag reordering. Example: deleting A from [A(key-a), B(key-b)] saves B with key-a. Reordering swaps the two keys. This contradicts the promised preservation of existing item keys. It can break anything that tracks Portable Text items by _key, such as revision diffs or custom renderers. Preserve the original Portable Text key with each row through repeater operations, separately from any UI-only key. Generate a new key only for genuinely new buttons. Add regression tests for deleting and reordering a multi-button group.
Reviewer disagreement: Anthropic rated this a non-blocking suggestion because Buttons.astro is unaffected. OpenAI rated it needs_fixing because of the key-identity risk. Anthropic also suggested narrowing the doc comment if positional reuse is kept deliberately.
| import { PortableTextEditor } from "../../src/components/PortableTextEditor"; | ||
| import { render } from "../utils/render"; |
There was a problem hiding this comment.
[needs fixing] These new relative imports omit the .js extensions required by the root AGENTS.md ESM import convention. The neighboring unsupported-block test already uses the required spelling.
| import { PortableTextEditor } from "../../src/components/PortableTextEditor"; | |
| import { render } from "../utils/render"; | |
| import { PortableTextEditor } from "../../src/components/PortableTextEditor.js"; | |
| import { render } from "../utils/render.js"; |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
upgrade-emdash
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-ai-search
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
Registering a built-in buttons definition through the existing Block Kit plugin-block path is the right approach. Imported WordPress button groups now open a real form, /buttons inserts a group, and a plugin's own buttons definition still takes precedence. Both reviewers approved with no findings. Both earlier findings are resolved at this head. Each button's _key is carried in _editorKey through edits, deletions and reordering, and the test imports now use the required .js extensions. A regression test covers deleting a button. The reviewers did not add a drag-reorder test, but they reasoned that reordering moves the same row objects, so the key stays with its button. The review was static; tests, builds and lint were not run.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
github-actions
Bot
removed
the
ci:run
danielmlr
left a comment
danielmlr
left a comment
There was a problem hiding this comment.
Hi Kevin, I found 3 things to sort out before merge:
-
Groups saved with the old "Enter URL..." box from #4313 still have that URL as their
id. The card keeps showing it, with Copy URL and Open in new tab, and every save writes it back. I think clearing it when the buttons block is saved would tidy those up. It probably needsid: "", since the edit path keeps the oldidwhen none comes through. -
If
buttonsisn't a list, the form says the value stays as it is, but saving turns it into[]. Returning it unchanged fromnormalizeButtonItemsin that case would keep that promise. -
German already translates
Outlineas "Gliederung" for the document outline, so the style select would show that. A context on the style labels, liket({ message: "Outline", context: "button style" }), would keep them apart.
Small one: an item_label on the repeater would show "Add button" instead of "Add item".
emdashbot
Bot
added
review/needs-review
github-actions
Bot
added
review/approved
…ng Buttons blocks
github-actions
Bot
added
review/needs-rereview
|
Thanks @danielmlr , all four are in 935361f: saving a buttons group now writes id: "", a non-list buttons value is kept as is, the style labels have a button style context, and the repeater says "Add button". Each has a test that fails without the change |
There was a problem hiding this comment.
Registering a built-in buttons definition through the existing Block Kit plugin-block path is the right approach. Imported WordPress button groups now open a real form, /buttons inserts a group, and a plugin's own buttons definition still takes precedence. Both reviewers approved with no findings. The earlier findings stay resolved at this head. Each button's key travels with its row through edits, deletions and reordering, and the test imports use .js extensions. This head also clears a stray group-level id on save, with tests and a changeset update. Both reviews were static; tests, builds and lint were not run.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
github-actions
Bot
added
review/approved
Resolve the conflict with main's editable Buttons block (#4595) in PortableTextEditor: keep this branch's key-based plugin block lookup and main's button key and item handling, and restore the local isRecord the button helpers use, since this branch moved the converter out of the editor.
What does this PR do?
WordPress imports turn
core/buttonsgroups intobuttonsblocks, and the site renders them, but the admin had no editor for them. Since #4314 the card says "No editor is available for this block type", so imported buttons could only be kept or deleted, and there was no way to add a new group.This registers a built-in Buttons block in the Portable Text editor using the existing Block Kit form:
/buttonsin the slash menu inserts a new groupSaving writes the shape
Buttons.astro/Button.astroalready render (_type: "button"items with_key). Existing item keys and any extra item properties are kept, and nothing is written to the parent'sid. If a plugin registers its ownbuttonsblock, the editor keeps using the plugin's definition.Out of scope: an open-in-new-tab option.
Button.astrohas notargetsupport yet, so a toggle would save data the site ignores.Closes #4313
Type of change
Your connection to this change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI assistance disclosure
Screenshots / test output
Before: the imported group shows "No editor is available for this block type. Its content is preserved." with only a delete action.
After, the imported group in the editor:
After, editing the group: layout select and the list of buttons with text, URL and style fields:
New
tests/editor/buttons-block-editor.test.tsx: editing an imported button and inserting via/buttonsboth fail onmainand pass with this change. The fixture inunsupported-block.test.tsxmoved frombuttonsto an unknowncolumnstype so it still covers the no-editor fallback. Full admin suite: 284 files, 4137 tests passed.