Skip to content
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/39443 attribute filters #39685

Merged
merged 4 commits into from Aug 16, 2023
Merged

Add/39443 attribute filters #39685

merged 4 commits into from Aug 16, 2023

Conversation

louwie17
Copy link
Contributor

Submission Review Guidelines:

Changes proposed in this Pull Request:

This adds the not visible and not filterable icons to the attributes list. It also adds a Use as filter option to the edit modal for global attributes. This is disabled by default as suggested by @jarekmorawski in this comment: #39443 (comment) as this is not supported by the 'Filter by Attribute' block yet.

Closes #39443 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Requires feature flag: product-variation-management

  1. Before enabling the New product Editor create a variable product in the legacy editor with at-least one local variable product.
  2. Load this branch and enable the new product editor under WooCommerce > Settings > Advanced > Features
  3. Install the WooCommerce Beta tester plugin and enable the product-variation-management feature under Tools > WCA Test Helper > Features
  4. Go to Products > All products and click edit on the previously created variable product.
  5. Go to the Variations tab and a yellow notice should be shown in the Variation Options field, warning about the local attribute being present.
  6. Feel free to dismiss this, it should disappear (even on refresh).
  7. Notice how the Local variable also has an not filterable icon with a tooltip that say's: 'Custom attribute. Customers can’t filter or search by it to find this product'
  8. Now add a global attribute (if you don't have one already) to variable options, and click Edit
  9. A Use as filter checkbox should be present ( selected but disabled ) with a tooltip saying: 'Check to allow customers to search and filter by this option in your store.'
  10. Now de-select Visible to customers and click Update
  11. A not visible icon should be present now with a tooltip saying: 'Not visible'
  12. Go to the Organization tab and scroll down to Attributes
  13. Add both a global and local attribute, the same icons should be present here ( Local showing the Not filterable icon, and both showing the Not visible icon if Visible to customers has been un-checked.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@louwie17 louwie17 requested a review from a team August 11, 2023 13:21
@github-actions github-actions bot added focus: react admin [team:Ghidorah] package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Aug 11, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2023

Hi @mdperez86,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

&-help-icon {
position: absolute;
right: -2px;
bottom: 0px;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made absolute so that the help icon overlaps the other icon a little bit.
They are also different colours which is why I treated them as separate icons instead of one.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2023

Test Results Summary

Commit SHA: 5343134

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 52s
E2E Tests1950015021023m 4s

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.

@louwie17 louwie17 force-pushed the add/39443_attribute_filters branch 2 times, most recently from 2559520 to 829a0e8 Compare August 14, 2023 15:23
Copy link
Contributor

@mdperez86 mdperez86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@louwie17 I see a wrong tooltip text when editing an attribute option
image

@louwie17
Copy link
Contributor Author

@louwie17 I see a wrong tooltip text when editing an attribute option

@mdperez86 that should be correct, did you try to click on the question mark, that should cause the popup to happen. It is slightly odd behaviour, but I believe that is what is intended (same as the other info icons ). The premise is that the Tooltip shows Help and once you click on it, it shows the actual info text.

@mdperez86
Copy link
Contributor

@louwie17 I see a wrong tooltip text when editing an attribute option

@mdperez86 that should be correct, did you try to click on the question mark, that should cause the popup to happen. It is slightly odd behaviour, but I believe that is what is intended (same as the other info icons ). The premise is that the Tooltip shows Help and once you click on it, it shows the actual info text.

That feels no accessible at all, specially in mobile devices. But it's ok if those are the requirements.

Copy link
Contributor

@mdperez86 mdperez86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as expected. Nice job @louwie17 and thanks for clarification!

@jarekmorawski
Copy link

@mdperez86 Regarding accessibility, the popover (the white one) should work the same as a regular tooltip, like in [Mozilla's guidelines](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tooltip_role). I assume that'd mean no additional tooltip on hover (the dark one).

@mdperez86 mdperez86 merged commit c11b116 into trunk Aug 16, 2023
26 checks passed
@mdperez86 mdperez86 deleted the add/39443_attribute_filters branch August 16, 2023 18:49
@github-actions github-actions bot added this to the 8.1.0 milestone Aug 16, 2023
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Aug 16, 2023
@nigeljamesstevenson nigeljamesstevenson added status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Aug 16, 2023
samueljseay pushed a commit that referenced this pull request Aug 18, 2023
* Add not filterable and not visible icons to attribute list

* Fix types

* Add changelogs

* Fix lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Product Block Editor] Use attributes as filter
4 participants