-
Notifications
You must be signed in to change notification settings - Fork 219
Product Collection - Add Created
filter in inspector controls
#11562
Product Collection - Add Created
filter in inspector controls
#11562
Conversation
This commit introduces the ability to filter products within the Product Collection block by a specified time frame. The changes include: - A new 'timeFrame' property added to the DEFAULT_QUERY constant in constants.ts, initialized as null, allowing for the storage of time frame data. - Creation of a new component `CreatedControl` in created-control.tsx that provides UI elements for selecting a time frame filter. - Inclusion of `CreatedControl` in the Product Collection Inspector Controls. - Expansion of the ProductCollectionQuery interface in types.ts to include a 'timeFrame' attribute. - Addition of the 'timeFrame' parameter handling within the ProductCollection PHP class to construct and execute the date query based on the provided time frame. The addition of the time frame filter offers enhanced flexibility in presenting products and allows users to dynamically segment their product lists based on product creation dates.
Created
filter in inspector controls
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
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: +250 B (0%) Total Size: 1.54 MB
ℹ️ View Unchanged
|
This commit includes a refactoring that changes the initialization and reset values for the `timeFrame` property from `null` to `undefined`. This standardization affects the constants, type definitions, and the handling of the `timeFrame` property in both the inspector controls and the PHP backend.
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.
This is testing well and the code looks good too. I left one minor comment, but approving on my behalf.
<ToggleGroupControlOption | ||
value={ ETimeFrameOperator.IN } | ||
label={ __( 'IN', 'woo-gutenberg-products-block' ) } | ||
/> | ||
<ToggleGroupControlOption | ||
value={ ETimeFrameOperator.NOT_IN } | ||
label={ __( | ||
'Not in', | ||
'woo-gutenberg-products-block' | ||
) } | ||
/> |
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.
@Aljullu, I've referenced the design from the provided screenshot; however, I'm unable to locate it in Figma. @kmanijak, could you please share the Figma design's URL? 🙂
To ensure consistency, we could:
- Capitalize all letters: IN & NOT IN
- Or capitalize only the first letter: In & Not in
@manospsyx What do you think? 🙂
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'd say to use capitalized letters since the "In" might not look great, but I'd wait for Mano's input!
Another nit: It would be better to use the _x()
function with context instead of the generic __()
. These words are small, making it difficult to understand what is actually being translated.
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'd probably:
- capitalize both (preferably via CSS; it's not great to rely on localizable strings for styling), and
- use a slightly smaller font size, perhaps by 5-10%, or 1-2px :)
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.
Another nit: It would be better to use the _x() function with context instead of the generic __(). These words are small, making it difficult to understand what is actually being translated.
Makes sense to me. What do you think about the following contexts?
_x('Not in', 'Exclude products created within a certain time frame', 'woo-gutenberg-products-block');
_x('In', 'Include products created within a certain time frame', 'woo-gutenberg-products-block');
I can make the changes once we finalize the context text.
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.
It's a good practice for translators to have context when dealing with short phrases. There is no set rule for when to use the _x() function, but I believe this is a valid reason to use it.
That said, It's also advisable to provide context for other instances here like "Stack" or "Grid".
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.
@xristos3490 Makes sense. Let's try to figure out what text we can use for context 🙂
_x('Not in', 'Product Collection query operator', 'woo-gutenberg-products-block');
_x('In', 'Product Collection query operator', 'woo-gutenberg-products-block');
This doesn't seem relevant anymore as we have changed the text to "Within" & "Before". What alternative text you would suggest? Here is my suggestion:
_x('Within', 'filter option for products created within a certain time frame', 'woo-gutenberg-products-block');
_x('Before', 'filter option for products created before a certain time', 'woo-gutenberg-products-block');
What do you think?
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.
That said, It's also advisable to provide context for other instances here like "Stack" or "Grid".
Makes sense. What do you think about the following?
_x('Stack', 'layout style for displaying products in a single column', 'woo-gutenberg-products-block');
_x('Grid', 'layout style for displaying products in multiple columns', 'woo-gutenberg-products-block');
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.
In my opinion, providing an exact description of the context could result in excessive wordiness. A simple term such as "Product Collection query operator" should suffice for translators to comprehend the intended meaning. However, I do not have a strong preference, so I leave it up to you to decide. :)
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 don't have a strong opinion either. I will go ahead and add "Product Collection query operator". Thanks 🙏🏻
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 work, @imanish003!
I also left a few comments!
<ToggleGroupControlOption | ||
value={ ETimeFrameOperator.IN } | ||
label={ __( 'IN', 'woo-gutenberg-products-block' ) } | ||
/> | ||
<ToggleGroupControlOption | ||
value={ ETimeFrameOperator.NOT_IN } | ||
label={ __( | ||
'Not in', | ||
'woo-gutenberg-products-block' | ||
) } | ||
/> |
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'd say to use capitalized letters since the "In" might not look great, but I'd wait for Mano's input!
Another nit: It would be better to use the _x()
function with context instead of the generic __()
. These words are small, making it difficult to understand what is actually being translated.
Moving to 11.6.0 |
This commit changes the column reference in the date query from 'post_date' to 'post_date_gmt'. This update ensures that the product collection filtering is based on Coordinated Universal Time (UTC) rather than local time, which can be affected by Daylight Saving Time (DST) shifts. The modification will lead to more consistent and reliable behavior across different time zones and during DST changes.
The following adjustments have been made: - Introduced a constant `uppercaseStyle` to store the `{ textTransform: 'uppercase' }` style. - Applied `uppercaseStyle` to both the 'IN' and 'NOT IN' toggle options to ensure label text is consistently uppercase. - Updated the 'Not in' label text to uppercase ('NOT IN') to match the newly applied style. These changes ensure that the toggle labels align with the design guidelines that call for uppercase styling in control elements.
This commit updates the internationalization (i18n) for the Product Collection query operators in the 'Created' control component of the WooCommerce Blocks plugin. It replaces the '__' function with '_x' for translation and provides context comments for better translation handling. This improvement enhances the localization of the query operators for better multilingual support.
What
Users can now easily limit their search results to products created within specific time frames. Here is how the UI of new filter looks like:
The code changes include:
CreatedControl
in created-control.tsx that provides UI elements for selecting a time frame filter.CreatedControl
in the Product Collection Inspector Controls.Fixes #11357
Why
The motivation behind these changes is to provide store owners with more flexibility and precision when displaying products. By filtering products based on their creation date, store owners can create dynamic collections that highlight new products or inventory changes over specific periods.
Testing Instructions
Please consider any edge cases this change may have, and also other areas of the product this may impact.
Case 1: Normal
Demo Video:
Screen.Recording.2023-11-08.at.11.32.13.am.mov
Case 2: Resetting the value of the Created filter
Case 3: Ensuring older blocks aren't broken (To be tested by developers only)
add/11357-product-collection-filters-creation-date
Screenshots or screencast
WooCommerce Visibility
Required:
Checklist
Required:
[type]
label or a[skip-changelog]
label.Conditional:
[skip-changelog]
label is not present).Changelog