Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Search Result Counts Match #2037

Merged
merged 13 commits into from
Jan 3, 2020
Merged

Conversation

supernova-at
Copy link
Contributor

@supernova-at supernova-at commented Dec 18, 2019

Description

There are two places we show the count of a search result: in the autocomplete dropdown and on the search page itself. Unfortunately, these two places were calculating their values differently and so sometimes did not match.

This PR:

  • Updates the way the autocomplete dropdown was calculating its value
  • Updates the search results page to add pagination so the number of items matches the number of results indicated

Related Issue

Closes JIRA-97.

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Perform a search for "Tops Dresses"
  2. Note how many items the autocomplete dropdown says have been found
  3. Press <Enter> to execute the search and continue to the search page
  4. Note how many items the search page says have been found
  5. See that the two numbers match

Screenshots / Screen Captures (if appropriate)

Autocomplete

Screen Shot 2019-12-18 at 11 03 07 AM

Search Page

Screen Shot 2019-12-18 at 11 03 16 AM

Checklist

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Dec 18, 2019
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Dec 18, 2019

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: JIRA-97.

Generated by 🚫 dangerJS against 71aa5d2

tjwiebell
tjwiebell previously approved these changes Dec 18, 2019
Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

Impressive 👏 I shouldn't be surprised how easy it was to re-use the Pagination component here, but I'm slightly surprised. Nice work!

@dpatil-magento
Copy link
Contributor

@supernova-at Observations -

  1. Get results for say "Tops" and be on page 2 or 3. >> Now enter new search which has only 1 page results say "VT10" >> Issue - No results are displayed.
  2. Get results for say "Tops" and be on page 2 >> Now enter new search which has multiple search products say "Dresses" >> Issue - Dresses results displayed but with page 2 in focus instead of page 1.

tjwiebell
tjwiebell previously approved these changes Dec 19, 2019
Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

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

LGTM 👍

This is giving me some minor PTSD, the logic seems more complicated that it needs to be, but I recall encountering the same problems with Category/ProductList. In the twenty minutes I've spent trying to simplify, I've only created infinite loops, so I'm good with this 😆

@dpatil-magento
Copy link
Contributor

@supernova-at Same issue exists when selecting from suggestion box.
Enter "Tops" be on page 3 of search results >> Click back in search text field > Select "tops in Tops" suggested search. Issue - Search results does not load as it has products only for 2 pages and we searched from page 3.

@supernova-at
Copy link
Contributor Author

@dpatil-magento great job!

Fixed in 19d17c6.

@dpatil-magento
Copy link
Contributor

QA pass. @tjwiebell You can merge once it passes the review.

@tjwiebell tjwiebell merged commit 5ed8de7 into develop Jan 3, 2020
@tjwiebell tjwiebell deleted the supernova/97_search_result_count branch January 3, 2020 17:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
pkg:peregrine pkg:venia-ui version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants