-
Notifications
You must be signed in to change notification settings - Fork 219
Product Collection: Transfer layout options from Toolbar to Inspector controls #10922
Product Collection: Transfer layout options from Toolbar to Inspector controls #10922
Conversation
In this update, the layout options for the Product Collection block are transferred from the Toolbar to the Inspector controls. Below is the breakdown of the changes: 1. **Constants Update** - `LayoutOptions` enumeration has been imported into `constants.ts`, facilitating a more structured approach to managing layout types (grid and stack). - The default display layout type has been updated from 'flex' to reference `LayoutOptions.GRID`. - The `getDefaultQuery` function now uses the `getDefaultValueOfInheritQueryFromTemplate` utility to set the default `inherit` value. (This is mainly done to fix a bug) 2. **Display Layout Control Removal** - The `display-layout-control.tsx` file has been removed, discontinuing the previous method of layout management. 3. **New Layout Options Control** - A new component `LayoutOptionsControl` has been introduced in the `layout-options-control.tsx` file, utilizing the experimental `ToggleGroupControl` and `ToggleGroupControlOption` components from the WordPress package to provide a more intuitive layout selection experience. - The `types.ts` file has been updated to define the `LayoutOptions` enum, effectively mapping 'flex' to 'GRID' and 'list' to 'STACK'. 4. **Inspector Controls Update** - In `inspector-controls/index.tsx`, the obsolete `DisplayLayoutControl` has been replaced with the new `LayoutOptionsControl`, integrating it into the `ProductCollectionInspectorControls` component. - The `BlockControls` wrapper has been removed, and layout options have been relocated to the Inspector controls, presented as a toggle group within the ToolsPanel. 5. **Inherit Query Control Modification** - The `inherit-query-control.tsx` file sees a change in the reset value for the `inherit` query attribute to employ a default value which fix one bug. These changes aim to streamline the user experience by relocating the layout options from the Toolbar to the Inspector controls, offering a centralized location for block settings. Leveraging an enum for layout options fosters code readability and maintainability. Do note that the update uses experimental components, hence it would be prudent to keep an eye on potential alterations or deprecations in upcoming WordPress releases.
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/product-collection/inspector-controls/layout-options-control.tsx
assets/js/blocks/product-collection/toolbar-controls/display-layout-toolbar.tsx assets/js/blocks/product-template/edit.tsx |
Size Change: +214 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
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.
Code looks great! Well done @imanish003 ! Clean and easy to ready and well split!
I just have a couple of concerns regarding the actual experience:
- [Minor] When I add the block, I see an animation on the inspector control selecting “Grid” (the default), is that unavoidable?
- I'm a a bit concerned about the names of these options and about the disregard of responsive controls. The “GRID” layout effectively is stacked on smaller viewports, making the distinction effectively meaningless (it even feels weird, as when I switch between them, the only thing that happens is a slight change in the margins: so it doesn't even feel consistent). Personally, I'm concerned about our lack of support for mobile use-cases, when they are such a big part of the eCommerce ecosystem. I fear that we, as developers, testing on our desktop devices, often forget of the needs of real world merchants.
I have already expressed my concern during my rotation for Ghetto Gastro (p6q8Tx-3ei-p2), in which I found that to be a huge limitation of FSE tools.
I'm approving this PR as I think this is part of a bigger conversation, but I wanted to flag it to you.
Thank you for initiating this conversation 🙏🏻 I will merge this PR, and we can continue our conversation here 💪🏻 |
I stupidly hadn't thought of changing the order. Honestly—and it might be my LTR bias—I think getting the default option on the left would make more sense. It's tough since Shaun rotated away, but maybe we can check how Gutenberg does it in other situations? Are the default values usually on the left?
I think it is “expected” as in our “intended” behavior. I don't think, personally, it is expected by the user for “grid” and “stack” to be the same thing. But, then again, it's a bigger topic and definitely the screenshot you sent and the issue you mentioned would pave the way for better UX! 🙌 |
I think it typically makes sense to keep the default option on the left, but it's more of a UX decision. I found one example in Gutenberg where the Default option is on the right. Take a look at the 'Orientation' setting in the 'Stack' block below: Screen.Recording.2023-09-18.at.5.08.19.PM.mov
💯 |
What
This PR Transfer layout options from the Toolbar to the inspector controls. Here is what result looks like:
Part of woocommerce/woocommerce#42225
Why
Testing Instructions
Please consider any edge cases this change may have, and also other areas of the product this may impact.
Case 1: Normal flow
Case 2: Reset button
Create a new post and add the product collection block.
Open the sidebar settings and note the absence of a 'RESET' option in the layout menu, indicated in the screenshot:
Change the layout from 'Grid' to 'Stack'. Open the settings menu again to find the 'RESET' button now visible, as shown here:
Verify that clicking the 'RESET' button changes the layout back to 'Grid'. Similarly, clicking the 'Reset all' button should revert the layout to 'Grid'.
Screenshots or screencast
WooCommerce Visibility
Required:
Checklist
Required:
[type]
label or a[skip-changelog]
label.Conditional:
[skip-changelog]
label is not present).Changelog