Add IframeEditor component to product editor #37570

joshuatf · 2023-04-05T00:28:01Z

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:

This PR adds in an IframeEditor for use in pages to allow us to edit specific sections, such as the product description, within a nested editor.

Note that some follow-ups will be needed and we'll need some design input moving forward (cc @jarekmorawski):

Back/exit button
Callback functions for onChange, onClose, etc.
Implementation of this block within the description
Updates to the design of this component

Closes #37266 .

How to test the changes in this Pull Request:

Either check out the branch add/37266-test OR add the IframeEditor component manually to a page by importing it from @woocommerce/product-editor.
Navigate to Tools -> WCA Test Helper -> Features and enable the new product blocks editing experience
Navigate to Products -> Add new (or where you manually added the new component)
Check that blocks can be added and blocks styles exist
Make sure you can resize the editor as needed

github-actions · 2023-04-05T00:28:19Z

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-04-05T00:29:32Z

packages/js/product-editor/src/components/iframe-editor/editor-canvas.tsx

@@ -0,0 +1,63 @@
+/**


Note that this component and ResizableEditor both exist under the edit-site package in Gutenberg but are not exported.

We should consider making those consumable, but that may require significant changes on their end to make it work for our use case.

Maybe we should add a TODO: comment here, or something else explicit, that mentions it being copied from the edit-site package.

Good idea! Since you're already AFK today, I'm going to go ahead and merge this though to unblock future work and we can add this comment in one of the following PRs.

joshuatf · 2023-04-05T00:30:45Z

packages/js/product-editor/src/components/iframe-editor/iframe-editor.scss

+    height: 100%;
+    width: 100%;
+    overflow: hidden;
+    position: fixed;


This is currently fixed, but we may want to slot fill this somewhere else into the page to position absolutely and prevent overlap with the existing editor.

More investigation needed here to determine the right solution.

github-actions · 2023-04-05T23:50:08Z

Test Results Summary

Commit SHA: 340d4b9

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 51s
E2E Tests	187	0	0	10	0	197	14m 31s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

louwie17

Hey @joshuatf, overall nice work on this, I did manage to get it working, but ran into several issues ( I left comments inline for those ).
Let me know what you think.

louwie17 · 2023-04-06T08:13:21Z

packages/js/product-editor/src/components/iframe-editor/editor-canvas.tsx

+		<Iframe
+			head={
+				<>
+					<EditorStyles />


It seems like this takes a styles prop, or relies on one for me. It errors and blank screens without providing one.
Maybe this has been recently added?

I did not manage to hit the error, but since we're not really passing any extra styles, I think we can remove it until we want to pass settings down.

louwie17 · 2023-04-06T08:14:12Z

packages/js/product-editor/src/style.scss

 @import 'components/header/style.scss';
 @import 'components/images/editor.scss';
 @import 'components/block-editor/style.scss';
+@import 'components/modal-editor/style.scss';


This style sheet doesn't exist, which breaks the build

Whoops! Must be some funkiness after the rebase/rename. Removed.

louwie17 · 2023-04-06T08:16:41Z

plugins/woocommerce/src/Admin/Features/BlockEditorFeatureEnabled.php

 		if ( ! empty( $post_type_object->template ) ) {
 			$editor_settings['template']     = $post_type_object->template;
 			$editor_settings['templateLock'] = ! empty( $post_type_object->template_lock ) ? $post_type_object->template_lock : false;
+			$editor_settings['__unstableResolvedAssets'] = gutenberg_resolve_assets_override();


The gutenberg_resolve_assets_override is not available on a standalone WP version, and requires the Gutenberg plugin to be enabled in order to work.
We should probably wrap this in a function_exists and maybe create our own if this is required? Not sure if this would be added to WP 6.3 for example, but that be a relatively long wait.

It seemed that we also rely on this function for some of the experimental components coming from @wordpress/block-editor, or maybe they were also provided by the Gutenberg plugin.

Copied this function over until it's available. 55c9ccf

codecov · 2023-04-15T00:50:23Z

Codecov Report

Merging #37570 (12bf0f4) into trunk (c226fa1) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

❗ Current head 12bf0f4 differs from pull request most recent head 340d4b9. Consider uploading reports for the commit 340d4b9 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             trunk   #37570     +/-   ##
==========================================
- Coverage     51.6%    51.6%   -0.0%     
  Complexity   17260    17260             
==========================================
  Files          429      429             
  Lines        79928    79928             
==========================================
- Hits         41213    41212      -1     
- Misses       38715    38716      +1

Impacted Files	Coverage Δ
...ugins/woocommerce/includes/class-wc-post-types.php	`1.1% <0.0%> (ø)`
...ludes/tracks/events/class-wc-products-tracking.php	`0.0% <ø> (ø)`

... and 1 file with indirect coverage changes

louwie17

Nice work @joshuatf, this tested well and looks good as is, it's nice that the components you needed from edit-site are also relatively small, so its not like we are copying over a huge amount of code for the short term.
I look forward to seeing this integrated.

I did leave one minor comment about mentioning in code where the copied over components are coming from, but that can be a follow up as well.

louwie17 · 2023-04-18T07:47:06Z

packages/js/product-editor/src/components/iframe-editor/editor-canvas.tsx

@@ -0,0 +1,63 @@
+/**


Maybe we should add a TODO: comment here, or something else explicit, that mentions it being copied from the edit-site package.

joshuatf requested a review from a team April 5, 2023 00:28

joshuatf self-assigned this Apr 5, 2023

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

joshuatf commented Apr 5, 2023

View reviewed changes

joshuatf mentioned this pull request Apr 5, 2023

[SPIKE] Investigate usage of standalone block editor within Modal #37290

Closed

2 tasks

joshuatf force-pushed the add/37266 branch from 5a18ff7 to 46f2577 Compare April 5, 2023 23:43

louwie17 reviewed Apr 6, 2023

View reviewed changes

joshuatf force-pushed the add/37266 branch from 46f2577 to 12bf0f4 Compare April 15, 2023 00:17

joshuatf added 18 commits April 17, 2023 10:03

Add initial modal editor

f1b8fc5

Add in iframe editor styles

a6850cd

Convert components to TS

efa23d8

Sync dependencies

fea29d5

Remove unused components and props

4bd941f

Set min height to avoid jumpiness during resize

10aee3e

Fix up private APIs dependency version

bb10988

Update ModalEditor name to IframeEditor

931a154

Add changelog entry

41a4f9d

Remove IframeEditor from details block

68ca5aa

Update frozen lock file

0601305

Add support for missing gutenberg assets function

e7e1f78

Fix lock file after rebase

366a717

Use default editor settings when none are provided

c2a61be

Remove currently unused editor styles

22c6670

Remove unused private apis package

b7f8f95

Fix php lint errors

fcc1d61

Remove unused import

603aafa

Pin keycodes version

6480e02

joshuatf force-pushed the add/37266 branch from 12bf0f4 to 6480e02 Compare April 17, 2023 17:08

joshuatf added 2 commits April 17, 2023 10:28

Remove another unused import from testing

199a462

Add WC changelog entry

340d4b9

joshuatf mentioned this pull request Apr 18, 2023

Iframe editor callback functions #37771

Closed

3 tasks

louwie17 approved these changes Apr 18, 2023

View reviewed changes

joshuatf merged commit ebe879d into trunk Apr 18, 2023

joshuatf deleted the add/37266 branch April 18, 2023 17:52

github-actions bot added this to the 7.8.0 milestone Apr 18, 2023

joshuatf mentioned this pull request Apr 20, 2023

Iframe editor should be placed inside of a modal #37898

Closed

3 tasks

Add IframeEditor component to product editor #37570

Add IframeEditor component to product editor #37570

Uh oh!

Conversation

joshuatf commented Apr 5, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Uh oh!

github-actions bot commented Apr 5, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results Summary

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

louwie17 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Apr 5, 2023 •

edited

Loading

codecov bot commented Apr 15, 2023 •

edited

Loading