aligent / magento-stockists-module

GNU General Public License v3.0
1 stars 1 forks source link

Future pagebuilder expansion of stockist attributes #18

Open brettlaishley opened 2 years ago

brettlaishley commented 2 years ago

https://github.com/aligent/magento-stockists-module/blob/1654e3d7dcd5ccfac3a30761689e4b2f6d57dc56/view/adminhtml/layout/stockists_index_edit.xml#L8

I have just spent 4h investigating a problem with the old Gallery stockist attribute from V1 of the Stockists module, and don't want this investigation time to be lost should future upgrades or specific client uses to the stockist module require a pagebuilder attribute.

The only change required is to add this line of code:

<update handle="editor"/>

I'll create a PR if desired, but I'm not sure if you want to introduce this if there are no current clients using pagebuilder within the stockist module?

aligent-lturner commented 2 years ago

@brettlaishley if there's a bug in v1 and you've got a simple fix, please create a PR targeting the develop-1.x branch

brettlaishley commented 2 years ago

Will do @aligent-lturner, there is a direct bug in v1, but on top of that it will also affect v2 should the description attribute ever be customised for pagebuilder in the future, which only requires:

        <field name="description" formElement="wysiwyg" sortOrder="50" template="ui/form/field">
            <argument name="data" xsi:type="array">
                <item name="config" xsi:type="array">
                    <item name="source" xsi:type="string">page</item>
                    <item name="wysiwygConfigData" xsi:type="array">
                        <item name="pagebuilder_button" xsi:type="boolean">false</item>
-                       <item name="is_pagebuilder_enabled" xsi:type="boolean">false</item>
+                      <item name="is_pagebuilder_enabled" xsi:type="boolean">true</item>

Do you think it's also worth targeting the main branch as well to future proof the module?

aligent-lturner commented 2 years ago

@brettlaishley yep - might as well plug the hole.