Skip to content

Add product management description #34961

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.

Sign up for GitHub

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

Jump to bottom
Merged
joshuatf merged 12 commits into from
Oct 18, 2022
Merged

Add product management description #34961

joshuatf merged 12 commits into from
Oct 18, 2022

Conversation

@joshuatf
Copy link
Contributor

@joshuatf joshuatf commented Oct 5, 2022

All Submissions:

Changes proposed in this Pull Request:

Adds a description field to the new product management experience.

Also adds context to the API request to fix up returned response data.

New editor:
Screen Shot 2022-10-05 at 10 11 07 AM

Old editor:
Screen Shot 2022-10-05 at 10 11 22 AM

How to test the changes in this Pull Request:

  1. Go to the new product management experience Products -> Add new (MVP)
  2. And add some content to the description field (note there are still some odd quirks around block selection in the editor that are unrelated to this PR)
  3. Save the content
  4. Refresh and make sure it works as expected
  5. Visit the old product edit page for the same product
  6. Note that content works as expected

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> run changelog add?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@joshuatf joshuatf requested a review from a team October 5, 2022 17:22
@joshuatf joshuatf self-assigned this Oct 5, 2022
joshuatf
joshuatf commented Oct 5, 2022
View reviewed changes
packages/js/data/src/products/resolvers.ts
const product: Product = yield apiFetch( {
path: `${ WC_PRODUCT_NAMESPACE }/${ productId }`,
path: addQueryArgs( `${ WC_PRODUCT_NAMESPACE }/${ productId }`, {
context: 'edit',
Copy link
Contributor Author

@joshuatf joshuatf Oct 5, 2022

Choose a reason for hiding this comment

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

context is important for the returned data type as the default view context formats returned data in a way that is not ideal for editing.

Allowing context to be changed would better, but I think we can forego that since this data store will most likely be replaced with the CRUD data store generator in the near future.

@github-actions github-actions bot added focus: react admin package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Oct 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022 •
edited
Loading

Test Results Summary

Commit SHA: 9fed06b

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests18800201900m 41s
E2E Tests186003018915m 33s
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.

mdperez86
mdperez86 suggested changes Oct 7, 2022
View reviewed changes
@joshuatf
Copy link
Contributor Author

joshuatf commented Oct 10, 2022

Thanks for the review, @mdperez86!

I think you made some good suggestions here. I attempted several of them, but noted how they might negatively impact performance and introduce bugs to the current experience.

It might be worth us exploring whether or not we should internally parse blocks back and forth to html instead of the consumers handling this. I'll post this to Slack soon to get some more discussion around this.

I think a lot of these suggestions are still useful and worth exploring, but since this PR does not need those features, I'd recommend we open up follow-up issues to try and implement some of those enhancements. What do you think?

@joshuatf joshuatf force-pushed the add/gb-text-editor-native-toolbar branch 3 times, most recently from bc4589d to af30df8 Compare October 12, 2022 20:22
Base automatically changed from add/gb-text-editor-native-toolbar to trunk October 12, 2022 21:15
@joshuatf joshuatf force-pushed the add/product-management-description branch from 03930d9 to fef29c1 Compare October 17, 2022 19:58
mdperez86
mdperez86 previously approved these changes Oct 17, 2022
View reviewed changes
mdperez86
mdperez86 previously approved these changes Oct 18, 2022
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2022

New hook, template, or database changes in this PR

New hooks:

  • file: /plugins/woocommerce/src/Admin/Features/NewProductManagementExperience.php
    • NOTICE - 'enqueue_block_editor_assets' introduced in 7.1.0: Enqueue any block editor related assets.

@github-actions github-actions bot added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Oct 18, 2022
@joshuatf joshuatf merged commit 3c66810 into trunk Oct 18, 2022
@joshuatf joshuatf deleted the add/product-management-description branch October 18, 2022 15:55
@github-actions github-actions bot added this to the 7.2.0 milestone Oct 18, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2022

Hi @joshuatf, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

@datgausaigon
Copy link

datgausaigon commented Oct 29, 2022 •
edited
Loading

Cool update but how about:

  • "Add Media" (Images, Videos)
  • "Table (HTML Table)"

in the description?

Thanks

@github-actions github-actions bot mentioned this pull request Dec 15, 2022
Closed
@github-actions github-actions bot mentioned this pull request Jan 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mdperez86 mdperez86 mdperez86 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joshuatf joshuatf

Labels

package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. release: highlight Issues that have a high user impact and need to be discussed/paid attention to.

Projects

None yet

Milestone

7.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@joshuatf @datgausaigon @mdperez86 @AnnaMag