refactor: put all yard patterns related code together#71
Conversation
laravdiemen
force-pushed
the
refactor/yard-patterns
branch
from
4483e2d to
55acc51
Compare
| \add_action('init', function () { | ||
| if (! apply_filters('yard::gutenberg/enable-patterns', true)) { | ||
| return; | ||
| } | ||
|
|
||
| $patternPostType = new PatternPostType(); | ||
| $patternPostType->boot(); | ||
|
|
||
| \add_action('admin_menu', [$this, 'registerAdminMenu']); | ||
| \add_action('enqueue_block_assets', [$this, 'enqueueMenuAssets']); | ||
| \add_filter('parent_file', [$this, 'highlightTaxSubMenu']); | ||
| \add_action('admin_init', [$this, 'registerAsBlockPatterns']); | ||
| }); |
There was a problem hiding this comment.
Om die apply_filters werkend te hebben moet het in een action zitten want direct in de boot werkt het niet. Ik weet alleen niet of dit nou de mooiste manier is. Als iemand nog een suggestie heeft om het mooier te krijgen, dan hoor ik dat graag.
There was a problem hiding this comment.
Een hook in een hook is inderdaad smelly. Ik zou deze init hook weghalen en een aparte functie maken:
private function isYardPatternsEnabled() {
return apply_filters('yard::gutenberg/enable-patterns', true);
}En in in elk van de functies van deze class het checken doen:
if (! $this->isYardPatternsEnabled()) {
return;
}
...There was a problem hiding this comment.
Moet je wel deze:
$patternPostType = new PatternPostType();
$patternPostType->boot();
naar een aparte functie doen, en wellicht wat omschrijven (zodat het in de init hook staat hier)
There was a problem hiding this comment.
Ik heb het aangepast, wel iets anders. Ik sla het op in isEnabled en die roep ik dan aan. Ik kreeg jouw voorstel om een of andere manier niet werkend maar dit is hetzelfde principe.
laravdiemen
requested review from
FreakyWizard,
JasperS20,
ShunLuk,
SimonvanWijhe,
WybeBosch,
Yannicvanveen,
YvetteNikolov,
Copilot,
dtakken,
hnccox-yard,
ictbeheer,
mvdhoek1 and
rivanuff
There was a problem hiding this comment.
Pull request overview
This PR reorganizes all Yard-pattern–related functionality into a dedicated YardPatterns module and fixes the yard::gutenberg/enable-patterns filter so it actually disables the Yard patterns feature when set to false.
Changes:
- Renames/moves the old menu/patterns responsibilities into
YardPatternsManagerand gates setup behindyard::gutenberg/enable-patterns. - Moves Yard “register as block patterns” logic out of
Patterns\PatternManagerintoYardPatterns\YardPatternsManager. - Updates build/webpack entrypoints and generated assets from
menu→yard-patterns(CSS + asset files).
Reviewed changes
Copilot reviewed 8 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Renames the SCSS entrypoint to produce style-yard-patterns.css. |
| src/YardPatterns/resources/scss/style.scss | Adds admin-menu icon sizing CSS for the Yard Patterns menu icon. |
| src/YardPatterns/resources/images/menu-logo.svg | Adds an SVG asset for the menu icon. |
| src/YardPatterns/YardPatternsManager.php | New central manager for Yard patterns: registers CPT/taxonomy, admin menu, assets, and block pattern registration (behind enable filter). |
| src/YardPatterns/PatternPostType.php | Moves PatternPostType into the YardPatterns namespace. |
| src/PluginServiceProvider.php | Switches provider bootstrapping from MenuManager to YardPatternsManager. |
| src/Patterns/PatternManager.php | Removes Yard-pattern registration logic, leaving only “disable external/plugin patterns” behavior. |
| build/yard-patterns.js | New/updated build artifact for the renamed entrypoint. |
| build/yard-patterns.asset.php | New build asset manifest for the renamed entrypoint. |
| build/style-yard-patterns.css | New compiled CSS for Yard Patterns admin menu styling. |
| build/style-yard-patterns-rtl.css | RTL variant of the new compiled CSS. |
| build/menu.asset.php | Removes obsolete asset manifest for the old menu entrypoint. |
Comments suppressed due to low confidence (2)
src/YardPatterns/YardPatternsManager.php:138
get_the_terms()can return aWP_Error. The current ternary treats any truthy value as an array and will pass aWP_Errorobject intoarray_map, causing warnings/TypeErrors. Handle the error case explicitly (e.g., treatWP_Erroras no terms).
src/YardPatterns/YardPatternsManager.php:164get_posts()uses the argument keyorder_by, but WordPress expectsorderby. As-is, sorting by title will be ignored and patterns will be returned in the default order.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| \add_action('init', [$this, 'setYardPatternsEnabled']); | ||
| \add_action('init', [$this, 'registerYardPatternsPostType']); | ||
| \add_action('admin_menu', [$this, 'registerAdminMenu']); | ||
| \add_action('admin_enqueue_scripts', [$this, 'enqueueMenuAssets']); |
There was a problem hiding this comment.
@YvetteNikolov je had deze pas aangepast naar enqueue_block_assets maar dan wordt het niet in de admin ingeladen, alleen als je in de gutenberg editor zit maar de styling die hier nodig is, is juist buiten de gutenberg editor nodig. Dus volgens mij is dit in dit geval wel de juiste hook.
There was a problem hiding this comment.
Je hebt gelijk, verander maar weer terug! Heb het te lomp veranderd zonder goed te kijken
Aan de werking is eigenlijk niks veranderd, behalve dat de filter
yard::gutenberg/enable-patternsnu wel daadwerkelijk goed werkt. Ik wil bijvoorbeeld bij Meedoen de Yard patronen uitschakelen omdat we daar via de code de patronen registeren. Toen ontdekte ik dat de code eigenlijk niet helemaal logisch was ingedeeld. InMenuManagerstond alles wat met Yard Patronen te maken had. InPatternsManagerstonden dingen die met Yard Patronen te maken hebben maar ook dingen die met de Wordpress patronen te maken hebben. Dus ik hebMenuManagerhernoemd naarYardPatternsManageren daar ook de relevante code van dePatternsManagerin gezet.