Site Editor: Don't register the Footnotes block #52823
Closed
Conversation
This file contains 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
Mamaduka
requested review from
ellatrix,
ramonjd,
tellthemachines and
ntsekouras
and removed request for
ajitbohra
Mamaduka
added
[Type] Bug
|
Why should it not work in the site editor? Is post content not editable in the site editor? |
ellatrix
reviewed
Jul 21, 2023
| export const init = () => { | ||
| // Would be good to remove the format and HoR if the block is unregistered. |
There was a problem hiding this comment.
The bit about the HoR can be remove, forgot that when we change how it's implemented.
ellatrix
reviewed
Jul 21, 2023
| export const init = () => { | ||
| // Would be good to remove the format and HoR if the block is unregistered. | ||
| registerFormatType( formatName, format ); |
There was a problem hiding this comment.
Regarding the comment above, this does register the format when the block is registered, but I feel like it should be tied closer to block type registration AND reregistration since these two depend on each other. Any ideas?
There was a problem hiding this comment.
Maybe we can use side-effect and blocks.registerBlockType register format when footnotes. But I don't see a corresponding unregister filter in the blocks package.
There was a problem hiding this comment.
What do folks think? Is the absence of a blocks.deregisterBlockType hook a blocker?
|
Size Change: +274 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
I don't think we have a way to differentiate if a user is editing post content or a template, at least not outside the |
Mamaduka
commented
Jul 21, 2023
Comment on lines
115
to
118
| const coreBlocks = __experimentalGetCoreBlocks().filter( | ||
| ( { name } ) => ! skippedBlocks.includes( name ) | ||
| ); | ||
| registerCoreBlocks( coreBlocks ); |
There was a problem hiding this comment.
@glendaviesnz, I couldn't think of the best fix at the moment.
I think integration tests initialize blocks more than once without proper editor state cleanup.
There was a problem hiding this comment.
the blocks were being cleaned up after each test here, but the footnotes block also adds a core/footnote format, and the formats were not being cleaned up - I have pushed an update which also removes any core formats that have been added - seems to fix the issue.
There was a problem hiding this comment.
Thank you, @glendaviesnz!
|
Flaky tests detected in 3587760. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5639630199
|
There was a problem hiding this comment.
How big a task would it be to add support for footnotes to templates?
This PR has the side effect of making it seem like footnotes are broken in pages:
If we have to choose between having footnotes broken in templates or in the pages view, I'd say it's likelier that they'll be used in the pages view so perhaps it's better to leave things as they are 😅
|
Not sure how valuable footnotes are for templates or template parts. But I agree that they should be kept intact for pages. Maybe we can unregister the format in the site editor and keep the block. |
ramonjd
approved these changes
Jul 25, 2023
There was a problem hiding this comment.
Functionality-wise, this is working as described and I think an improvement.
Is it something we could iterate on if the implementation isn't perfect?
Before
| Heading | Paragraph |
|---|---|
 |
 |
After
| Heading | Paragraph |
|---|---|
C |
 |
Edit: Actually, I completely missed the comment above
Maybe we can unregister the format in the site editor and keep the block.
If this works 🤞🏻
ellatrix
reviewed
Jul 25, 2023
There was a problem hiding this comment.
I'm not sure I like registering/unregistering the block to disable it. Ideally there should be a setting for block.json similar to the parent/ancestor key but for other context such as post type or post type support key. A post (type) is just a container as any other block and in the site editor you could be switching between these. So with this way, you'd have to register/unregister all the time.
|
An alternative fix: #52934 |
|
Closing in favor of #52934. |
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
PR disabled the Footnotes block for the Site Editor.
Why?
The footnotes don't work with templates.
How?
Move the footnotes format registration into the block init method and let the core block registrar handle both.
Testing Instructions