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
Add tab blocks to the blocks product editor #37174
Conversation
Test Results SummaryCommit SHA: 79dba45
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. |
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 much @joshuatf , and well done. Had a comment or two, and also have this in the console that I'm wondering if we should address:
"supports": { | ||
"align": false, | ||
"html": false, | ||
"multiple": false, |
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.
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.
Good catch. I never ran into this with this block already inserted, but regardless seems like something that should be enabled. Updated in ebfafab
|
||
function onClick( tabId: string ) { | ||
window.document.documentElement.scrollTop = 0; | ||
navigateTo( { |
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 use of this util 👍🏻
"keywords": [ "products", "tab", "group" ], | ||
"textdomain": "default", | ||
"attributes": { | ||
"id": { |
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.
Would it be useful to have support for disabled
via an attribute here?
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.
Definitely. Planned for the follow-ups to tabs, but I'd like for us to flesh out how we plan on "templating" different product types.
If we go with individual templates per product type, attributes may work well, but otherwise we may have to build logic directly into the block.
Thanks for the feedback, @joelclimbsthings! Addressed the comments.
I can create a follow-up for this. I don't see this being registered in WC blocks either, but they use the same category 🤔 We might need to double check before adding it to avoid duplicate categories. |
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 @joshuatf , changes look good and everything is working well. 🚢
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## trunk #37174 +/- ##
==========================================
- Coverage 46.7% 46.7% -0.0%
Complexity 17191 17191
==========================================
Files 429 429
Lines 64845 64862 +17
==========================================
Hits 30275 30275
- Misses 34570 34587 +17
|
All Submissions:
Changes proposed in this Pull Request:
This adds a
tab
block for use in the new product editor that is treated like a normal block. The tab button itself is slotfilled above the editor, while the selected tab is set using context.This does not address the following, which will be added in follow-up PRs:
Closes (part of) #37096 .
How to test the changes in this Pull Request:
add/37096-test
for initially hydrated sample tab blocks&tab=shipping
is in the URL) and note that shipping is initially selectedOther information:
pnpm --filter=<project> changelog add
?FOR PR REVIEWER ONLY: