fix (design library): remove from inserter #3587
Conversation
WalkthroughDesign Library block settings now include Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Editor as WP Editor
participant DesignSettings as Design Library settings
participant Disabled as disabled-blocks logic
Editor->>DesignSettings: Load block metadata/settings
DesignSettings-->>Editor: settings (includes supports.inserter = false)
Editor->>Disabled: apply metadata normalization
Disabled-->>Editor: leave inserter alone if already defined
Editor->>Editor: Build inserter catalog
Note right of Editor #dff0d8: Design Library block omitted from inserter
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/block/design-library/index.js (1)
39-48: Add a nullish guard when spreading supports.Minor hardening to avoid a TypeError if settings.supports is missing in some integration path.
addFilter( `stackable.design-library.settings`, `stackable/design-library/inserter`, settings => { return { ...settings, supports: { - ...settings.supports, + ...( settings?.supports ?? {} ), inserter: false, }, } } )Optional: If you want a static, zero-runtime alternative, set inserter: false in block.json (supports.inserter) instead of using a filter. (developer.wordpress.org)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/block/design-library/index.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (2)
src/block/design-library/index.js (2)
39-48: This correctly hides the block from the Inserter (and related UIs).Setting supports.inserter to false is the documented way to hide a block from the Inserter, transforms menu, and Style Book. (developer.wordpress.org)
39-48: Filter Application Not Found – Manual Verification RequiredI couldn’t locate any
applyFilters('stackable.design-library.settings', …)invocation in the registration path, nor anyregisterBlockType( … 'design-library' )calls that consume this filter. Without anapplyFilterscall running after this module loads, the filter you’ve added will never take effect. Please:
- Confirm where and when your block registration for the design library occurs.
- Add an
applyFilters('stackable.design-library.settings', settings)wrapper around the registration settings in that code path, ensuring this module is imported beforehand.
src/block/design-library/index.js
Outdated
| import { addFilter } from '@wordpress/hooks' | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Importing addFilter is appropriate; please confirm bundling/external mapping.
Looks good. Verify that @wordpress/hooks is either listed as a dependency or mapped as wp.hooks in your bundler to avoid runtime resolution issues.
🏁 Script executed:
#!/bin/bash
# Ensure @wordpress/hooks is available via deps or externals.
echo "Checking package.json for @wordpress/hooks…"
fd -a package.json | xargs -I{} jq -r '
.dependencies["@wordpress/hooks"] // empty,
.devDependencies["@wordpress/hooks"] // empty
' {} 2>/dev/null | sed '/^$/d' || true
echo -e "\nSearching build config for externals mapping to wp.hooks…"
rg -nP -C2 'externals|wp\.hooks|@wordpress/hooks' --type js --type ts --type tsxLength of output: 504
🏁 Script executed:
#!/bin/bash
# Search the entire repository (all file types) for externals mapping to wp.hooks or @wordpress/hooks
echo "Scanning for externals declarations related to @wordpress/hooks or wp.hooks…"
rg -nC2 'externals|wp\.hooks|@wordpress/hooks' .Length of output: 147287
🏁 Script executed:
#!/bin/bash
# Check if @wordpress/scripts is used for bundling
echo "Checking for @wordpress/scripts in dependencies/devDependencies…"
jq -r '
.dependencies["@wordpress/scripts"] // "not found",
.devDependencies["@wordpress/scripts"] // "not found"
' package.json
# Show package.json scripts to see build commands
echo -e "\npackage.json scripts:"
jq '.scripts' package.json
# Look for common bundler configuration files
echo -e "\nSearching for bundler config files…"
fd webpack.config.js || echo "No webpack.config.js found"
fd rollup.config.js || echo "No rollup.config.js found"
fd vite.config.js || echo "No vite.config.js found"Length of output: 3480
🏁 Script executed:
#!/bin/bash
# Scan webpack configuration files for externals mapping of @wordpress/hooks → wp.hooks
echo "Scanning .config directory for externals mappings…"
rg -nP 'externals\s*:' -C3 .configLength of output: 851
🏁 Script executed:
#!/bin/bash
# List the first 50 entries under .config to locate externals definitions
echo "Listing .config directory recursively (top 50 entries)…"
find .config -maxdepth 2 -type f | head -n 50
echo -e "\nSearching for 'externals =' definitions in .config files…"
rg -nP 'externals\s*=' -C3 .config
echo -e "\nSearching for 'stackableExternals' definitions in .config files…"
rg -nP 'stackableExternals' -C3 .configLength of output: 2952
🏁 Script executed:
#!/bin/bash
# Search for mapping of @wordpress/hooks in externals.js
echo "Inspecting externals.js for @wordpress/hooks mapping…"
rg -nC3 "@wordpress/hooks" .config/externals.js || echo "No direct mapping found"Length of output: 221
Add externals mapping for @wordpress/hooks
We’ve confirmed that
@wordpress/hooksis declared in package.json dependencies (version ^3.2.2).- Both webpack configs in
.config/webpack.config.dev.jsand.config/webpack.config.prod.jsspread in the sharedexternalsobject from.config/externals.js.
However, there’s currently no entry for @wordpress/hooks in .config/externals.js, so imports like
import { addFilter } from '@wordpress/hooks'will be bundled rather than resolved against the global wp.hooks at runtime.
Please update .config/externals.js by adding a mapping for the Hooks package, for example:
const externals = {
// existing mappings…
'@wordpress/hooks': [ 'wp', 'hooks' ],
wp: 'wp',
lodash: 'lodash',
stackable: 'stackable',
// …
};Because both dev and prod webpack configs use:
externals: {
...externals,
...stackableExternals,
},this change will ensure addFilter/applyFilters etc. resolve to window.wp.hooks.
🤖 Prompt for AI Agents
In src/block/design-library/index.js around lines 16 to 17: imports from
"@wordpress/hooks" are currently being bundled because .config/externals.js
lacks a mapping for that package; update .config/externals.js to add an
externals entry mapping '@wordpress/hooks' to the global wp.hooks (e.g. add a
key '@wordpress/hooks' with value ['wp','hooks'] alongside the existing
wp/lodash/stackable mappings) so both dev and prod webpack configs that spread
the shared externals will resolve addFilter/applyFilters against window.wp.hooks
at runtime.
bfintal
left a comment
bfintal
left a comment
There was a problem hiding this comment.
We can just set inserter to false from the json
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/disabled-blocks.js (1)
53-55: Fix operator-precedence bug:!binds before===, makinginserteralways falseThe current expressions negate the value before comparison, effectively returning
falsein typical cases. Compare directly againstBLOCK_STATE.HIDDEN.Apply:
- inserter = ! disabledBlocks[ metadata.name ] === BLOCK_STATE.HIDDEN + inserter = disabledBlocks[ metadata.name ] !== BLOCK_STATE.HIDDEN- inserter = ! disabledBlocks[ metadata[ 'stk-block-dependency' ] ] === BLOCK_STATE.HIDDEN + inserter = disabledBlocks[ metadata[ 'stk-block-dependency' ] ] !== BLOCK_STATE.HIDDENAlso applies to: 58-60
♻️ Duplicate comments (1)
src/disabled-blocks.js (1)
6-6: Ensure externals mapping for@wordpress/hooks(wp.hooks) existsImport relies on bundler externals. Verify
.config/externals.jsmaps@wordpress/hooks→['wp','hooks']. See prior review thread.
🧹 Nitpick comments (3)
src/disabled-blocks.js (1)
95-103: Confirm precedence: should user-hidden blocks override explicitsupports.inserter?With this guard, blocks that set
supports.inserter: truewon’t be hidden by user settings. If user-config should win, consider overriding unlesssupports.inserter === false.Potential approach:
- if ( typeof metadata.supports.inserter === 'undefined' ) { - metadata.supports.inserter = inserter - } + if ( metadata.supports.inserter === false ) { + // honor explicit false +} else { + metadata.supports.inserter = inserter +}src/block/design-library/index.js (2)
25-28: Don’t overwritemetadata.supports; merge itPreserve any supports defined in
block.json.Apply:
- supports: { - stkSaveBlockStyle: false, - inserter: false, // Always hide design library from block inserter - }, + supports: { + ...( metadata.supports || {} ), + stkSaveBlockStyle: false, + inserter: false, // Always hide design library from block inserter + },
16-36: Add a quick UI test to prevent regressionsVerify the block is absent from: (a) block inserter, (b) slash inserter. Also ensure existing content with the block remains editable.
I can draft a minimal Playwright/E2E spec asserting both inserters exclude this block—want me to add it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/block/design-library/index.js(1 hunks)src/disabled-blocks.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: build
🔇 Additional comments (2)
src/disabled-blocks.js (1)
101-103: Guarded assignment preserves explicit block settings — LGTMOnly setting
supports.inserterwhen undefined avoids clobbering block-defined intent.src/block/design-library/index.js (1)
27-27: Hide from inserter viasupports.inserter: false— LGTMThis aligns with the objective to remove the Design Library from the inserter.
bfintal
deleted the
fix/3581-remove-design-library-from-block-inserter
branch
fixes #3581
Summary by CodeRabbit