Allow the user to delete the description #39229
Conversation
|
Hi @mattsherman, 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: 4373e02
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. |
mattsherman
left a comment
mattsherman
left a comment
There was a problem hiding this comment.
Works as expected.
I think the naming can be clarified a bit to make it more clear that the description is only cleared if it is "empty".
| * Internal dependencies | ||
| */ | ||
|
|
||
| function clearDescription( blocks: BlockInstance[] ) { |
There was a problem hiding this comment.
How about calling this clearDescriptionIfEmpty?
There was a problem hiding this comment.
Yep I was thinking about this or something similar but I could not find a good name for this. I also considered IfEmpty but since blocks list is not empty it was a little confusing for me. It should be something like remove the first block if its content is empty and it's just the only one block in the block list. Can you resume this in a couple of words?
| // By default the blocks variable always contains one paragraph | ||
| // block with empty content, that causes the desciption to never | ||
| // be empty. The next line removes the default block to keep | ||
| // the description empty. |
There was a problem hiding this comment.
I think that if you rename the function as suggested above, you can also move this comment inside the function, as it isn't needed here with a more specific function name and the comment really helps to better understand the function implementation.
mattsherman
left a comment
mattsherman
left a comment
There was a problem hiding this comment.
Thanks for making those adjustments, @mdperez86 -- looks good! Nice work.
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #38853
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Changelog entry
Significance
Type
Message
Comment