Forms: Hide Jetpack block collection from form editor inserter#47390
Forms: Hide Jetpack block collection from form editor inserter#47390
Conversation
Remove the "Jetpack" block collection from the block inserter when editing forms, since form field blocks are organized into their own granular categories. The collection is saved and restored when leaving the form editor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
This PR updates the Jetpack Forms editor experience by hiding the global “Jetpack” block collection in the block inserter when editing jetpack_form posts, while restoring it when leaving the form editor so other editors remain unchanged.
Changes:
- Added a block-collection utility to remove and later restore the Jetpack block collection via the
core/blocksstore. - Wired the utility into the form editor subscription lifecycle (remove on enter/first setup, restore on leave).
- Added unit tests for the new utility and a changelog entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| projects/packages/forms/src/form-editor/utils/block-collection.ts | New utility to remove/restore the Jetpack block collection, storing collection metadata between transitions. |
| projects/packages/forms/src/form-editor/index.tsx | Calls the new remove/restore functions when entering/leaving the form editor. |
| projects/packages/forms/tests/js/form-editor/utils/block-collection.test.js | New Jest tests validating the utility’s basic behavior. |
| projects/packages/forms/changelog/fix-hide-jetpack-category-from-form-editor | Patch changelog entry documenting the inserter change for forms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/forms/src/form-editor/utils/block-collection.ts
Outdated
Show resolved
Hide resolved
projects/packages/forms/src/form-editor/utils/block-collection.ts
Outdated
Show resolved
Hide resolved
| describe( 'removeJetpackBlockCollection', () => { | ||
| it( 'should remove the jetpack collection when it exists', () => { | ||
| mockGetCollections.mockReturnValue( { | ||
| jetpack: { title: 'Jetpack', icon: 'jetpack-icon' }, | ||
| } ); | ||
|
|
||
| removeJetpackBlockCollection(); | ||
|
|
||
| expect( mockRemoveBlockCollection ).toHaveBeenCalledWith( 'jetpack' ); | ||
| } ); | ||
|
|
||
| it( 'should not call removeBlockCollection when no jetpack collection exists', () => { | ||
| mockGetCollections.mockReturnValue( {} ); | ||
|
|
||
| removeJetpackBlockCollection(); | ||
|
|
||
| expect( mockRemoveBlockCollection ).not.toHaveBeenCalled(); | ||
| } ); | ||
|
|
||
| it( 'should not call removeBlockCollection when getCollections is unavailable', () => { | ||
| mockGetCollections.mockImplementation( () => { | ||
| throw new Error( 'not a function' ); | ||
| } ); | ||
|
|
||
| // Override select to return an object without getCollections | ||
| const dataModule = require( '@wordpress/data' ); | ||
| const originalSelect = dataModule.select; | ||
| dataModule.select = () => ( {} ); | ||
|
|
||
| removeJetpackBlockCollection(); | ||
|
|
||
| expect( mockRemoveBlockCollection ).not.toHaveBeenCalled(); | ||
|
|
||
| dataModule.select = originalSelect; | ||
| } ); | ||
| } ); | ||
|
|
||
| describe( 'restoreJetpackBlockCollection', () => { | ||
| it( 'should restore a previously removed collection', () => { | ||
| mockGetCollections.mockReturnValue( { | ||
| jetpack: { title: 'Jetpack', icon: 'jetpack-icon' }, | ||
| } ); | ||
|
|
||
| removeJetpackBlockCollection(); | ||
| jest.clearAllMocks(); | ||
|
|
||
| restoreJetpackBlockCollection(); | ||
|
|
||
| expect( mockAddBlockCollection ).toHaveBeenCalledWith( 'jetpack', 'Jetpack', 'jetpack-icon' ); | ||
| } ); | ||
|
|
||
| it( 'should not call addBlockCollection when no collection was previously saved', () => { | ||
| restoreJetpackBlockCollection(); | ||
|
|
||
| expect( mockAddBlockCollection ).not.toHaveBeenCalled(); | ||
| } ); | ||
|
|
||
| it( 'should only restore once after removal', () => { | ||
| mockGetCollections.mockReturnValue( { | ||
| jetpack: { title: 'Jetpack', icon: 'jetpack-icon' }, | ||
| } ); | ||
|
|
||
| removeJetpackBlockCollection(); | ||
| jest.clearAllMocks(); | ||
|
|
||
| restoreJetpackBlockCollection(); | ||
| restoreJetpackBlockCollection(); | ||
|
|
||
| expect( mockAddBlockCollection ).toHaveBeenCalledTimes( 1 ); | ||
| } ); | ||
| } ); |
There was a problem hiding this comment.
Tests cover the happy path, but they don’t exercise the defensive branches where dispatch('core/blocks') lacks removeBlockCollection/addBlockCollection. Since the implementation treats those actions as optional, add tests that verify (a) no module state is saved when removal can’t happen, and (b) saved state isn’t discarded when restore can’t run, so the operations remain reversible.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@claude review this pr. |
|
Claude finished @enejb's task in 1m 47s —— View job PR Review
|
| Area | Status |
|---|---|
| Core implementation logic | ✅ Correct — Copilot's two concerns are already addressed |
Integration in index.tsx |
✅ Looks good |
| Test: happy-path coverage | ✅ Good |
| Test: defensive branch coverage | |
| Changelog | ✅ Appropriate |
The implementation is solid. The only meaningful gap is the test coverage for the defensive dispatch-API-missing branches. If you want to ship without those tests, the risk is low (the production code is correct), but it would leave those guards unverified.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/packages/forms/tests/js/form-editor/utils/block-collection.test.js
Outdated
Show resolved
Hide resolved
projects/packages/forms/tests/js/form-editor/utils/block-collection.test.js
Outdated
Show resolved
Hide resolved
projects/packages/forms/src/form-editor/utils/block-collection.ts
Outdated
Show resolved
Hide resolved
Code Coverage SummaryCoverage changed in 2 files.
1 file is newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
Replace the require()-based mock override with mockSelect.mockReturnValue() to be compatible with the ESM test environment used in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use `await jest.unstable_mockModule()` and dynamic `await import()` instead of `jest.mock()` which relies on `require()` unavailable in the ESM test environment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Would it altenratively work to check if we're at form custom post type editor and not register in first place, or use |
Add tests verifying that: - removeJetpackBlockCollection does not save state when removeBlockCollection is missing from dispatch - restoreJetpackBlockCollection preserves savedCollection when addBlockCollection is missing from dispatch, allowing a later restore to succeed Also clean up redundant jest.clearAllMocks() in beforeEach by extracting a shared resetMocks helper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| describe( 'block-collection', () => { | ||
| beforeEach( () => { | ||
| resetMocks(); |
There was a problem hiding this comment.
The beforeEach has duplicated jest.clearAllMocks() and repeated mockSelect.mockImplementation(...) calls. This makes the setup harder to follow and isn't needed to reset the mocks/module state; consider keeping a single clear + a single mockSelect setup, then calling restoreJetpackBlockCollection() (if desired) between them.
| resetMocks(); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Good call. I didn't know this existed. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| disableBlockDirectory(); | ||
| removeJetpackBlockCollection(); | ||
| } | ||
|
|
There was a problem hiding this comment.
removeJetpackBlockCollection() is only invoked inside the one-time categoriesSetUp block. If the Jetpack block collection is registered later in the load sequence (e.g., by projects/plugins/jetpack/extensions/editor.js importing ./shared/block-category), the initial removal can be a no-op and the collection may still appear in the inserter for jetpack_form. Consider re-attempting collection removal while isFormEditor is true until it actually succeeds (or tracking a separate collectionRemoved flag rather than piggybacking on categoriesSetUp).
| // Ensure the Jetpack block collection is removed even if it is | |
| // registered later in the load sequence. | |
| removeJetpackBlockCollection(); |

Before:

After:
Jetpack collection doesn't exist anymore.
Proposed changes:
jetpack_formpost type).Other information:
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Open the page editor.
Add a form. Select a variation.
Click the Editor form. Notice that Jetpack is not there in the block inserter.
Click the back button.
Notice that Jetpack is in the block inserter.
Open a Jetpack Form in the form editor (create or edit a
jetpack_formpost type).Open the block inserter.
Verify that the "Jetpack" collection label no longer appears in the inserter — form blocks should be grouped under their granular form categories (Basic, Contact Info, Choice, etc.) instead.
Navigate away from the form editor to a regular post/page editor.
Open the block inserter again and verify the "Jetpack" collection is visible as expected.
Changelog
🤖 Generated with Claude Code