-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Add ContentPreview component for previewing block content #37990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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: |
Test Results SummaryCommit SHA: 5b3a077
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
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @joshuatf this looks great, and tested well!
Glad this was relatively straightforward.
I did notice a couple issues with the editor (which I know isn't related to this PR, but thought I call them out):
- It looks like when I type some text and hit back some of the changes are not being reflected, I assume this is because of the
useDebounce, although this will probably be fixed if we make use of the buttons within the Modal. - There appears to be some missing styling still, for example when I add a button, the initial button styling is missing and I can't justify it to the right or to the left. This is reflected correctly on the front-end. The alignment issue mostly pertains to missing css that sets the block to
flex.
| 'li', | ||
| 'ol', | ||
| 'div', | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, Lourens!
It looks like when I type some text and hit back some of the changes are not being reflected, I assume this is because of the useDebounce, although this will probably be fixed if we make use of the buttons within the Modal.
I notice this even without debounce. I'll create an issue to look into this. I believe there may already be some level of debouncing occurring within the Gutenberg components.
There appears to be some missing styling still, for example when I add a button, the initial button styling is missing and I can't justify it to the right or to the left. This is reflected correctly on the front-end. The alignment issue mostly pertains to missing css that sets the block to flex.
Good catch! I'll create an issue around this as well.
I'm going to merge this one as-is since two issues are unrelated to this PR and I'll get some issues created to tackle both of these in follow-ups.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Adds a
ContentPreviewcomponent and hooks up the description with a preview area.Note that the preview area does not show video content currently. I'm not sure if this absolutely necessary to have and may create loading and/or render issues in the product form.
Closes #37900 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions: