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

[PNI] Search - update basic search behaviour #7649

Merged
merged 11 commits into from
Oct 28, 2021
Merged

[PNI] Search - update basic search behaviour #7649

merged 11 commits into from
Oct 28, 2021

Conversation

fessehaye
Copy link
Contributor

@fessehaye fessehaye commented Oct 20, 2021

Closes #7475 #7476

keep product filtering, and make sure that categories are always listing all products, with out-of-category products hidden through CSS by default (Right now categories are prefiltered to only show in-category products)
When the user starts searching:

  • “unhighlight” the category, if we're on a category
  • highlight the "all" category
  • present “search” results from the full PNI product set.

Sorting/ranking priority of the results:

  • Product name
  • Company name
  • Blurb

No changes to design

@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-2uycvn October 20, 2021 17:50 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 20, 2021 18:01 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 20, 2021 18:28 Inactive
@Pomax
Copy link
Contributor

Pomax commented Oct 20, 2021

Hm, interesting problem: it looks like search.js does not load until all images have loaded, which means you can't search for quite a while, particularly noticable when running a stage copy without S3 settings, which causes all product images to 404, and searchign doesn't work until all of those 404s have finished

Pomax
Pomax previously requested changes Oct 20, 2021
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
network-api/networkapi/wagtailpages/pagemodels/products.py Outdated Show resolved Hide resolved
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 04:26 Inactive
@fessehaye fessehaye requested a review from Pomax October 21, 2021 04:27
Pomax
Pomax previously requested changes Oct 21, 2021
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
@fessehaye fessehaye requested a review from Pomax October 21, 2021 21:13
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 21:13 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 22:04 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 22:17 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 22:30 Inactive
Pomax
Pomax previously requested changes Oct 21, 2021
package.json Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 22:47 Inactive
@fessehaye fessehaye requested a review from Pomax October 21, 2021 22:47
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 23:07 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 21, 2021 23:09 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 27, 2021 17:51 Inactive
Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

The checkbox is remembered on Firefox too now 👍 Thanks for the update Simon!

@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 27, 2021 18:21 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 27, 2021 18:22 Inactive
@kristinashu
Copy link

This is working super well!

Not a blocker but I noticed that if I'm in /entertainment and start typing in search, the content and nav updates to All (good), but the url doesn't update. Not sure if this is being done elsewhere and it isn't a blocker but maybe a follow up ticket?

Screen Shot 2021-10-27 at 11 19 47 AM

@Pomax Pomax self-requested a review October 27, 2021 18:56
@Pomax
Copy link
Contributor

Pomax commented Oct 27, 2021

@kristinashu that's intentional: searching doesn't change the URL you're on, but explicitly selecting a category does.

Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

This looks good as far as I can tell! some things we can iterate on, but let's file followup issues for that, for someone else to tackle in the future. I tested on the current stage copy using both en and fr locales and everything seemed to highlight correctly now.

fixing
@mofodevops mofodevops temporarily deployed to foundation-s-7475-searc-pblmts October 27, 2021 20:51 Inactive
Copy link
Contributor

@TheoChevalier TheoChevalier left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me too now, just highlighted one potential issue

network-api/networkapi/wagtailpages/pagemodels/products.py Outdated Show resolved Hide resolved
@Pomax Pomax temporarily deployed to foundation-s-7475-searc-pblmts October 28, 2021 16:05 Inactive
@Pomax Pomax merged commit 5312002 into pni-q3-2021 Oct 28, 2021
@Pomax Pomax deleted the 7475-search branch October 28, 2021 16:25
@Pomax Pomax mentioned this pull request Nov 2, 2021
Pomax pushed a commit that referenced this pull request Nov 2, 2021
* 7475 - update basic search behaviour
* 7476 - update search fields/ranking
Pomax pushed a commit that referenced this pull request Nov 2, 2021
* 7475 - update basic search behaviour
* 7476 - update search fields/ranking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants