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

Jump to bottom

In-app search filtering #40333

Merged
merged 28 commits into from Oct 4, 2023
Merged

In-app search filtering #40333

merged 28 commits into from Oct 4, 2023

Conversation

bgrgicak
Copy link
Contributor

@bgrgicak bgrgicak commented Sep 21, 2023 •
edited

Changes proposed in this Pull Request:

This PR adds support for selecting categories in the in-app search.
Category selector now accepts a type prop and loads different data for theme and extension search sections.

Clicking all extensions in the filter resulted with no data being returned, it was fixed by excluding the _all category from search requests.

How to test the changes in this Pull Request:

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

  1. Open the in-app marketplace
  2. Search for a term in the header (store is a good term)
  3. After loading you should see two sections (extensions and themes)
  4. Click on any category
  5. See both sections are loading, but just the clicked section has results (this will be changed later)
  6. Update your search term to something that won't produce results
  7. See that no results loaded
  8. Click on another section like Discover and see that the search term was removed

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

Update the marketplace search flow and fix category filtering for theme and extension search.

@bgrgicak bgrgicak added focus: react marketplace Issues relating to the React Marketplace team: Fire labels Sep 21, 2023
@bgrgicak bgrgicak self-assigned this Sep 21, 2023
@bgrgicak bgrgicak marked this pull request as ready for review September 21, 2023 12:25
@bgrgicak bgrgicak requested review from a team, Dan-Q and kdevnel and removed request for a team September 21, 2023 12:25
@github-actions
Copy link
Contributor

Hi @Dan-Q, @kdevnel,

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

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Sep 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2023 •
edited

Test Results Summary

Commit SHA: 013116d

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 57s
E2E Tests212007021920m 43s
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.

@bgrgicak
Copy link
Contributor Author

I just merged #40368 into this PR, so it may looks slightly bigger from what it should be.

Base automatically changed from add/18027-themes-to-in-app-search to feature/marketplace-themes September 26, 2023 04:08
Dan-Q
Dan-Q approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@Dan-Q Dan-Q left a comment

Choose a reason for hiding this comment

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

💛 Beautiful. Get it merged!

plugins/woocommerce-admin/client/marketplace/components/product-list-content/no-results.tsx
__(
"We didn't find any results for “%s”",
"We didn't find %1$s for “%2$s”",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how well this translates to other languages: "injecting" substrings like this can be tricky. I can't think of any specific examples, but there might exist languages where this doesn't work.

Like how in Japanese you use a different set of numbers when enumerating small quantities of physical things, depending on their shape, there could well be languages where the verb "find" conjugates differently depending on the object of the verb. Like if in English it was more-correct to say "We didn't find any themes" but "We didn't discover any extensions".

Probably not an issue, and I can't think of a "nice" solution anyway, but just mentioning in case you think it's worth doing anything about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we just have two strings, one for themes and one for extensions?

@bgrgicak bgrgicak closed this Sep 28, 2023
@kdevnel
Copy link
Contributor

kdevnel commented Sep 29, 2023

@bgrgicak was this close intentionally? Just checking in case it was accidental and still needs review.

@bgrgicak bgrgicak reopened this Sep 29, 2023
@bgrgicak
Copy link
Contributor Author

@bgrgicak was this close intentionally? Just checking in case it was accidental and still needs review.

No this was a mistake.

github-actions and others added 4 commits September 29, 2023 07:56
Add changefile(s) from automation for the following project(s): @wooc…
4174874 
…ommerce/product-editor, @woocommerce/dependency-extraction-webpack-plugin, @woocommerce/data, @woocommerce/components, @woocommerce/block-templates, woocommerce-beta-tester, woocommerce
@bgrgicak
Merge branch 'feature/marketplace-themes' into update/in-app-multiple…
c2c94e9 
…-category-filters
@bgrgicak
Merge branch 'update/in-app-multiple-category-filters' into add/marke…
486706a 
…tplace-theme-discover-section
Add changefile(s) from automation for the following project(s): wooco…
f6fbcdb 
…mmerce
kdevnel
kdevnel approved these changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@kdevnel kdevnel left a comment

Choose a reason for hiding this comment

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

This all checks out for me! Since a lot of my search tab code is based on this, everything makes sense!

@bgrgicak bgrgicak merged commit 7781a10 into feature/marketplace-themes Oct 4, 2023
23 of 25 checks passed
@bgrgicak bgrgicak deleted the update/in-app-multiple-category-filters branch October 4, 2023 10:54
@andfinally andfinally mentioned this pull request Oct 4, 2023
Merged
11 tasks
andfinally added a commit that referenced this pull request Oct 4, 2023
@andfinally
Support for themes in in-app marketplace.
c1a8f45 
Contains the changes from:

#40247
#40272
#40302
#40303
#40333
#40368
#40375
#40375
#40389
alopezari pushed a commit that referenced this pull request Oct 4, 2023
@kdevnel @andfinally @Dan-Q
Marketplace Themes: Feature Branch (#40159)
d8adecf 
* Support for themes in in-app marketplace.

Contains the changes from:

#40247
#40272
#40302
#40303
#40333
#40368
#40375
#40375
#40389

* `.woocommerce-marketplace__discover`: changed `align-items` `flex-start` to `stretch` to properly display products on large and very large viewports.

* Delete plugins/woocommerce/changelog/add-18026-marketplace-theme-cards

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/add-18027-themes-to-in-app-search

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/add-marketplace-theme-discover-section

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/update-in-app-multiple-category-filters

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/update-theme-no-result-style

Removing from feature branch before final review

* Add changefile(s) from automation for the following project(s): woocommerce

---------

Co-authored-by: And Finally <andfinally@users.noreply.github.com>
Co-authored-by: Dan Q <dan@danq.me>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Dan Q <danq@automattic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dan-Q Dan-Q Dan-Q approved these changes

@kdevnel kdevnel kdevnel approved these changes

Assignees

@bgrgicak bgrgicak

Labels
focus: react marketplace Issues relating to the React Marketplace plugin: woocommerce Issues related to the WooCommerce Core plugin. team: Fire
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bgrgicak @kdevnel @Dan-Q @andfinally