Skip to content

Conversation

@joshuatf
Copy link
Contributor

@joshuatf joshuatf commented May 8, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Fixes an issue where certain editor styles (such as buttons) were not being applied to the editor and iframe editor.

Before

Screen Shot 2023-05-08 at 9 50 14 AM

After

Screen Shot 2023-05-08 at 9 49 08 AM

Closes #38021.

How to test the changes in this Pull Request:

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

  1. Navigate to Tools -> WCA Test Helper -> Features and enable the new product blocks editing experience
  2. Navigate to Products -> Add new
  3. Click on the "Add description" button
  4. Add a button block
  5. Note that the button styles appear using your site default styles in the modal/iframe editor
  6. Close the modal
  7. Note that the preview also shows the same styles

@joshuatf joshuatf requested a review from a team May 8, 2023 16:52
@joshuatf joshuatf self-assigned this May 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

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

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Test Results Summary

Commit SHA: 4159528

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests26700202690m 52s
E2E Tests1880010019822m 8s

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.

Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, this is testing well!
There are a couple lint errors and I left one inline comment.

Also I noticed one issue we will have to create a follow up for, the content-preview doesn't render any of the inline css, so if I set the layout to center it should add in-line css of display: flex; justifyContent: center, but it only adds that as html comments:
<!-- wp:buttons {"layout":{"type":"flex","justifyContent":"center"}} -->

Maybe there is an alternative serialize function for this?

settings={ settings }
>
<EditorStyles styles={ settings?.styles } />
{/* <EditorStyles styles={ styles } /> */}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can be removed right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can! Good catch

@joshuatf
Copy link
Contributor Author

joshuatf commented May 9, 2023

Also I noticed one issue we will have to create a follow up for, the content-preview doesn't render any of the inline css, so if I set the layout to center it should add in-line css of display: flex; justifyContent: center, but it only adds that as html comments:

Good point. Will create another issue to address this.

Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@louwie17 louwie17 merged commit a587612 into trunk May 11, 2023
@louwie17 louwie17 deleted the fix/38021 branch May 11, 2023 14:43
@github-actions github-actions bot added this to the 7.8.0 milestone May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Iframe Editor: Missing button styles in editor

3 participants