-
Notifications
You must be signed in to change notification settings - Fork 219
Conversation
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/breadcrumbs/index.tsx
|
Size Change: +1.63 kB (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
"default": "small" | ||
} | ||
}, | ||
"supports": { |
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.
Based on the planned native breadcrumbs supports: https://github.com/WordPress/gutenberg/pull/32500/files
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.
Thank you for working on this block! 🎉
I checked it and it tests good 👍 I left minor comments, but except for them all looks good! I'm pre-approving then!
Thanks for the review here @kmanijak 🙌 ! Feedback addressed, awaiting design review regarding the copy for the block title and the icon to merge :) cc: @vivialice |
Hi @nefeline, one of the first usage of Breadcrumbs block will be in a blockified Product Archive template (PR). We realised there's a need for an alignment setting in Store Notices and Breadcrumbs block to be able to recreate the same current layout as current Classic Template (comment) Do you think you could add the setting within this PR or, in case you have other stuff to work on, I could help with that as I'm currently working on the same setting for the Store Notices block? |
Regarding icons - we merged Store Notices, Product Results Block and Catalog Sorting with temporary icons, so that won't be a blocker here as well. We'll batch update the icons once they're available. But I think we should merge with the correct block name. cc'ing: @dinhtungdu who documented the names and descriptions. What should be the block name: Store Breadcrumbs or WooCommerce Breadscrumbs (comment) |
Hi there @kmanijak !
I've added the attribute over here. The support was already enabled for |
Thank you for a really quick reaction and solution! 🙇
These are enough and also it's aligned with what was added to the Store Notices block 👍 Thank you again for this! 🙏 Let's sort out the naming and let's ship it! 🎉 |
Hey @nefeline ! If I am understanding correctly, we want feedback on the icon and the description text. The icon looks great to me 👌 I suggest a small amendment to the text by removing the 's' on 'locations':
I have a small design related request: is it possible to increase the margin around the '/' on the breadcrumb? Maybe double what it is at the moment? |
…y for the block description.
Hi @vivialice : thanks for the feedback!
Awesome! 🙌
✅ Done
At this moment, it is not possible as we are relying exclusively on what the core of WooCommerce outputs for the breadcrumbs. |
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.
I know this has already been merged, but I left a couple of questions/comments. Do you think you can take a look, @nefeline? I can open issues if that makes it easier. 🙂
Introduce the new Store Breadcrumbs block, allowing merchants to keep track of their locations within the store and navigate back to parent pages.
Fixes #8063
Screenshots
Breadcrumbs
Editor sidebar
Block inserter
@vivialice this icon was selected as it was the closest that resembles a breadcrumb I've found: looking forward to hearing your thoughts here and if a different one should be used instead!
Testing
Automated Tests
User Facing Testing
WooCommerce Visibility
Changelog