POC: Product template extensibility API #38657

joshuatf · 2023-06-08T17:59:27Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

POC for extending the new block templates for various product types. This PR adds in some quick works and helper functions to demonstrate how 3PD (and internally) we can extend templates.

I'll be posting a p2 early next week to discuss some of the decisions made with this and challenges we may face.

Follow-ups

If we go this route, there's quite a few things this PR does not address:

Migrating the remainder of the existing template
Validation of block/field data
Unit tests
Duplicate ID prevention
Remove private keys (properties like _order might be useful on the frontend though, so I'm a bit mixed on its removal if it doesn't cause other issues)
Make IDs mandatory - This mostly makes reference and adding child blocks or removing blocks faster
Guard against additions to the root template - Discussed inline below. I'm against completely walling this off, but setting guardrails may be appropriate if the majority of use cases don't require root block additions.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Check out this branch
Enable the new product blocks editor under WooCommerce -> Settings -> Advanced -> Features
Navigate to the Add Product page
Note that the product template loads correctly (this is currently limited to a couple tabs and fields that have been added for the POC)
Add this plugin to your site
Note that the custom radio field is added to the template

github-actions · 2023-06-08T17:59:55Z

Hi , @woocommerce/mothra

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

joshuatf · 2023-06-08T18:01:14Z

...ocommerce/src/Admin/Features/ProductBlockEditor/ProductTemplates/AbstractProductTemplate.php

+    /**
+     * Add a group to the template.
+     */
+    public function add_group( $args = array() ) {


Noting that I used the word group here instead of tab to decouple this from the UI element so that we have the option to change this in the future.

I like the idea behind this, but having "groups" and "sections" is confusing. Maybe "section" and "sub-section"? Or "group" and "sub-group"?

Or, even just "group" or "section" and they can be nested and how they appear depends on what level they are at.

I do like avoiding "tab", as on different devices, such as mobile, they might not look like tabs.

Great thought!

I've toyed around with the idea of category and having unlimited number of nested items (or referencing them as "sections" as you mentioned above which may be better; I don't have a strong opinion on the naming yet).

On one hand, this is really great since it opens the door for unlimited nesting (which we already have multiple levels with sections in the latest template).

On the other hand, it does remove some level of intent with how the things being added should behave. The idea of a "group" being that common items are grouped together while sections represent some kind of visual separation on the page. But these semantics could backfire on us in the future should we change the meaning and move to something where sections/groups were identical in meaning (e.g., a flyout menu that shows groups, then sections when drilled down).

I'll post on this more in the p2 post coming Monday, but love any more thoughts/feedback you have in this area.

joshuatf · 2023-06-08T18:02:49Z

...ocommerce/src/Admin/Features/ProductBlockEditor/ProductTemplates/AbstractProductTemplate.php

+     * Insert a block into the template tree.
+     */
+    private function insert_block( $parent, $block, $id ) {
+        if ( $parent === self::ROOT ) {


We don't currently guard against blocks being inserted at the root. We could do so to try and enforce use of tabs/sections at the top level, but this is pretty easy to work around and might break some use cases so I think leaving it as is may be the best move, but open for discussion.

I'd say to leave possible.

joshuatf · 2023-06-08T18:04:59Z

...commerce/src/Admin/Features/ProductBlockEditor/ProductTemplates/ProductTemplateInterface.php

+	/**
+     * General group ID.
+     */
+    const GENERAL_GROUP = 'group/general';


This does not feel like the best place for these, but unfortunately PHP versions <8.2 don't support constants in traits and I don't think these core-specific groups/sections/fields belong in the abstract class.

mattsherman · 2023-06-09T20:46:37Z

...s/woocommerce/src/Admin/Features/ProductBlockEditor/ProductTemplates/BaseProductTemplate.php

+     */
+    protected function add_general_group() {
+        $this->add_group(
+            array(


Minor, but perhaps the array shorthand syntax ([ 'foo' => 'bar' ]) could be used instead? I think it would make these templates more readable.

I was under the impression that the linter preferred the non-shorthand version, but looks like that may not be the case. Definitely happy to change that if so!

mattsherman · 2023-06-09T20:48:20Z

...s/woocommerce/src/Admin/Features/ProductBlockEditor/ProductTemplates/BaseProductTemplate.php

+            array(
+                'id'    => self::GENERAL_GROUP,
+                'title' => __( 'General', 'woocommerce' ),
+                'order' => 10,


Why is order necessary? It would be nice to get rid of these hardcoded orders if possible.

The order property for tabs is specific to a bug with rendering tabs on the frontend. See this PR for more context.

But in general, I do not think we're moving away from using order. While I originally liked not having an order property, this would create too much dependency and require too much knowledge of the existing template.

For example, imagine we had $template->insert_field_before( 'woocommerce/product-name-field', $my_field ). This requires the consumer to know the name woocommerce/product-name-field, prevents us from changing to a different block without breaking third party implementations, and if some of our templates did not include this field, the third party field would also not be included.

Additionally, it also creates a lot of issues with timing. If two plugins each insert at the same block, which should render first?

// PluginA $template->insert_field_before( 'woocommerce/product-name-field', $field_a ); // PluginB $template->insert_field_before( 'woocommerce/product-name-field', $field_b );

Whichever was loaded later will be the one adjacent to the name field. If these were on the same hook, the alphabetical order of the plugins would dictate which function runs first.

<ProductNameField /> // Must be added first in order to work <FieldB /> // Added after A, so inserted next to the product name field <FieldA /> // Added before B

Compare this to including an order property which has specific intent and removes the timing issues we see between various hooks or plugin execution order.

mattsherman · 2023-06-09T20:49:53Z

Left a few comments. Looking forward to seeing more high-level examples of how this might be used, as I think that will be valuable to review and explore before getting too far into the weeds with any particular implementation.

Nice work on this so far!

mattsherman · 2023-06-09T20:52:06Z

Note: I didn't rest the actual execution of this at all... not necessary at this point, in my opinion. :-)

joshuatf · 2023-09-12T02:29:28Z

The templating API is now merged 😄. Closing this concept.

joshuatf added 14 commits June 6, 2023 13:55

Add initial base product template abstraction

bffda2b

Handle insertion under nested blocks

f22245f

Cache location of items with ID for easier insertion of blocks

545b13b

Parse field args as array instead of separate arguments

84b6f92

Remove associative array keys from template

f5a6d2c

Add in blocks sorting

eac381e

Fix index reference

0911b45

Change tab to group to decouple from UI element

c7bb616

Fix up semantics of abstractions and traits

0d4d5bc

Fix summary field block array

1cb20e4

Register entire template instance instead of template layout only

c925f11

Make field functions public

895292e

Remove testing functions

4069fe3

Add constructor to template interface

e41e551

joshuatf requested a review from a team June 8, 2023 17:59

joshuatf self-assigned this Jun 8, 2023

github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Jun 8, 2023

joshuatf changed the base branch from trunk to add/38277 June 8, 2023 17:59

joshuatf commented Jun 8, 2023

View reviewed changes

mattsherman reviewed Jun 9, 2023

View reviewed changes

AnnaMag added the category: extensibility label Jul 13, 2023

adrianduffell removed the focus: react admin [team:Ghidorah] label Sep 7, 2023

joshuatf closed this Sep 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

POC: Product template extensibility API #38657

POC: Product template extensibility API #38657

joshuatf commented Jun 8, 2023 •

edited

github-actions bot commented Jun 8, 2023

joshuatf Jun 8, 2023

mattsherman Jun 9, 2023

mattsherman Jun 9, 2023

mattsherman Jun 9, 2023

joshuatf Jun 9, 2023

joshuatf Jun 8, 2023

mattsherman Jun 9, 2023

joshuatf Jun 8, 2023

mattsherman Jun 9, 2023

joshuatf Jun 9, 2023

mattsherman Jun 9, 2023

joshuatf Jun 9, 2023

joshuatf Jun 9, 2023

mattsherman commented Jun 9, 2023

mattsherman commented Jun 9, 2023

joshuatf commented Sep 12, 2023

POC: Product template extensibility API #38657

POC: Product template extensibility API #38657

Conversation

joshuatf commented Jun 8, 2023 • edited

Submission Review Guidelines:

Changes proposed in this Pull Request:

Follow-ups

How to test the changes in this Pull Request:

github-actions bot commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsherman commented Jun 9, 2023

mattsherman commented Jun 9, 2023

joshuatf commented Sep 12, 2023

joshuatf commented Jun 8, 2023 •

edited