A real v1 #1
Merged
A real v1 #1
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
|
Nice work! Hit a few snags while testing... Added the plugin to the v2 branch of WSK, but getting this error on error log |
|
Should we update the node version in the Update: |
|
Noticed the docker config in the plugin — do you think it's really necessary for testing? I think it might be overkill especially if we usually just copy paste the whole repo into a WP project's |
ALJ
approved these changes
Feb 16, 2023
| * @return void | ||
| */ | ||
| public static function register() { | ||
| Config::read(); |
There was a problem hiding this comment.
Have you considered other ways of defining configuration for the connection between plugin and theme? I ask because WordPress has the add_theme_support and get_theme_support functions to somewhat standardize the activation of feature flags.
For example, I can imagine a system where WSK does something like:
add_theme_support( 'ups-editorial-author-panel' );And, Ups Editorial Plugin reads that with:
if ( get_theme_support( 'ups-editorial-author-panel' ) ) {
Taxonomy\Author::register();
}The advantage is it keeps the "standard WP way" that might feel more friendly to others. The potential disadvantage is it introduces WP's weirdness, like it might run afoul of WP hook sequence firing; I haven't tested this in depth to know the extent of the potential problems.
There was a problem hiding this comment.
I'm happy to consider alternative ways to implement config but I also want to see how this is used and get a sense from other devs how they would like to manage feature flagging. As for my thinking behind the config files...
Ideally themes would individually remove features rather than need to add them, so I'm not sure that the add_theme_support() paradigm would work. We may be able to invert this and have themes use disable_theme_support(), but we'd have to look into that a bit.
A similar pattern I did consider was using hooks to handle feature support:
Theme code
add_filter( 'ups_editorial_author_panel', '__return_false' );Plugin
if ( apply_filters( 'ups_editorial_author_panel', true ) ) {
Taxonomy\Author::register();
}Ultimately while that is the pattern I see most commonly around enabling/disabling plugin features and is effectively the standard way that WordPress handles this type of thing, I don't love that method: it requires namespaced filter names, which can get cumbersome; I'm never confident on the best place for this code to get added to a theme; and it just isn't that clear as to what it's doing (it took me years to understand that the __return_false callback is a global function in core).
I like the simplicity of a singular config file that is scoped to a specific application that defines settings for that application - it's one of the ideas that I pulled over from how Craft CMS handles things. While it's not necessarily a standard WordPress pattern, it's simple enough to be understood by theme developers and robust enough where we can continue adding to it easily without having to create a bunch of individual filters for the plugin.
There was a problem hiding this comment.
I like the simplicity of a singular config file that is scoped to a specific application that defines settings for that application
That sounds good enough for me! It was helpful to hear your thinking around it, and that you did consider some other WP-native ways, too.
v2: Remove style panel on Image core block
V2 continuous integration
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
This PR refactors and updates the
ups-editorialplugin (previously,ups-blocks) to account for recent core updates and best practices. It also removes dependencies on any WordPress theme (most notably the WP Starter Kit) so as to fully function like a standalone plugin.The changes introduced in this plugin correlate to many changes introduced to the WP Starter Kit in that repo's v2.
This plugin offers the following features:
ups_authortaxonomy registered by the plugin.ups-editorial.phpand simply return an array of relevant configuration values (see the README in this branch for documentation on the specific configuration values).Changes from the previous version
Refactors the plugin's structure and namespacing, including a more organized approach to the features defined in PHP vs. those defined in JS.
Removes the custom
ups/imageblock. As WordPress continued to improve their editor experience and update their core blocks, the customizations intended by theups/imageblock became antiquated as core image block functionality became more configurable.Simplifies how JavaScript and stylesheets are built and loaded. Previously, there were a number of disparate JavaScript files and scss partials floating around, hooked into a custom webpack config file. This update removes the custom webpack config by routing all JavaScript functionality through a singular
src/index.jsfile, which is how thewp-scriptspackage expects the code to be organized.How to test
Download the code in the
v2branch of this repo and add it to an existing WordPress installation. (The plugin should work for any WordPress installation running the latest version of the Gutenberg editor, but thev2branch of Upstatement's WP Starter Kit is a good blank slate to test against.)In the plugin's directory, run
npm installto install dev dependencies, then runnpm run buildto compile the production assets. You'll also need to runcomposer installto generate the autoloader (this will also install some dev dependencies like linters). Note that the intention is for this plugin to be pulled into a WordPress environment via composer, so release versions of this package will be shipped with compiled assets and any relevant dependencies all ready to go. This configuration is happening in a separate PR - this one is primarily about the specific functionality of the plugin.Activate the plugin.
If your theme has a
ups-editorial.phpconfig file, delete it. This will ensure that all plugin functionality is active.Confirm that an
Authorstaxonomy exists for your posts. Confirm that the bylines functionality works as expected (it should continue to work as it did before):/wp-admin/edit-tags.php?taxonomy=ups_authors)Authorspanel when editing a post. You should also be able to reorder and delete the author terms added via this panel.Confirm that the
Article Topperpanel in the editor works as expected. You should be able to add topper content and switch between the different article topper variations.Confirm that the Cover, File, Gallery, Image Layout, Related Articles, Table, and Video blocks all work as expected and have the expected customizations (they should continue to work in the same way as before). Note that the Image Layout and Related Articles blocks are not core blocks and need to be defined in your theme - these are defined in the WP Starter Kit. Future work on this plugin may include a better way to handle these non-core blocks.
Confirm that the template methods work in your theme. See the
Theme APIsection of the README for this branch for documentation on which template methods are available.