[bugfix] Hide stale suggestions #2150

jimbo · 2020-02-07T18:35:54Z

Description

Fix a bug with Suggestions wherein stale results would continue to be displayed when the field had been cleared. The proper fix would entail resetting the query, such that called would be false and data would be undefined, but Apollo does not yet offer that functionality. Instead, this fix recognizes when the field has been cleared and incorporates that into the conditional rendering logic of Autocomplete and Suggestions.

This fix involves changes to the talon APIs, so it's probably a major. As a result, I've also taken the opportunity to have SearchBar obtain history and location from hook calls rather than render props.

Related Issue

PWA-361

Acceptance

Verification Stakeholders

@dpatil-magento

Specification

Verification Steps

Enter athena in the search input.
Verify that suggested categories and products appear.
Click the button to clear the search input.
Verify that suggestions are no longer displayed, but a prompt is.

Screenshots / Screen Captures (if appropriate)

Checklist

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

PWAStudioBot · 2020-02-07T18:43:22Z

	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: PWA-361.

Generated by 🚫 dangerJS against 229aa40

tjwiebell

Good improvement, always thought that was just want UX wanted 😂 A few tiny things to clean up and this is good to go.

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js

packages/peregrine/lib/talons/SearchBar/useSearchBar.js

tjwiebell

LGTM 👍

dpatil-magento · 2020-02-13T18:42:54Z

@jimbo I can still reproduce the issue in one case - Enter "Tops Dress" > Wait for suggestions and delete the entire input via delete key. Now click outside the textfield and click back in. Search suggestions are shown for "Tops"

https://i.gyazo.com/d2bffcb93e3404881ba00ca584570ad6.gif

tjwiebell · 2020-02-13T19:00:31Z

@jimbo I can still reproduce the issue in one case - Enter "Tops Dress" > Wait for suggestions and delete the entire input via delete key. Now click outside the textfield and click back in. Search suggestions are shown for "Tops"

Looked at this with Dev; looks like its easiest to reproduce by holding down backspace to remove the search value.

jimbo · 2020-02-14T17:01:50Z

@jimbo I can still reproduce the issue in one case - Enter "Tops Dress" > Wait for suggestions and delete the entire input via delete key. Now click outside the textfield and click back in. Search suggestions are shown for "Tops"

Looked at this with Dev; looks like its easiest to reproduce by holding down backspace to remove the search value.

This is so bizarre. I can replicate this with tops dresses but not with athena. I'll have to look into it.

sirugh · 2020-02-19T17:40:31Z

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js


    // run the query once on mount, and again whenever state changes
    useEffect(() => {
        if (visible && valid) {
            debouncedRunQuery({ variables: { inputText: value } });
+        } else if (called && !value && !visible) {
+            setCleared(true);


What's happening is that as you delete the text of tops dresses the last "valid" search text is top which is used as the debounced query. That explains why you see product suggestions for top which are based on the last search data, but you don't see any text in the <input> in <Category> suggestions.

I fixed this by moving the setCleared logic into the debounced run query. The last call that executes should control the UI. In this case, we don't need to validate input before the debounce, we can do that inside the debounce, and if it is invalid we just clear the view.

Signed-off-by: Stephen Rugh <[email protected]>

jimbo · 2020-02-19T18:15:17Z

@dpatil-magento I think this is fixed.

dpatil-magento · 2020-02-19T20:35:55Z

After clearing if I click back > stale results display for fraction of second and goes away.
https://gyazo.com/4d31b77f18f0c7f07b6b6c126b3dff4c

dpatil-magento · 2020-02-20T04:46:00Z

Now the valid search suggestions also not displaying.

jimbo · 2020-02-20T21:18:28Z

@dpatil-magento I've fixed that issue. Please give it another test.

Important: we had logic that was reading the value of query from the URL and writing it to the search field. This logic was flawed and had a negative effect on the UX anyway, so after discussing it with @soumya-ashok, I removed this logic. As a result, if you navigate directly to the search page, it will no longer prepopulate the search field.

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js

sirugh · 2020-02-20T21:59:12Z

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js

+    const debouncedRunQuery = useMemo(
+        () =>
+            debounce(inputText => {
+                runQuery({ variables: { inputText } });


This now runs with the last 3 characters in the input when you start deleting some text value. For example "top shirt" will search "top shirt" but when you delete that character by character it'll query for "top" since it is the last valid value on the way to an empty string. Note that unlike the earlier bug it won't display the results because the display state of the results is based on the validity of the current input, not the last searched input.

I would prefer we don't make an extra call for text that may never be searched but it's also a minor optimization so 🤷

sirugh

+1+1+1+1

dpatil-magento · 2020-02-21T00:25:52Z

@jimbo Looks good but still there is minor issue. Input dress wait for search suggestion to show, now clear the search field and enter Top > For fraction of seconds old search suggestions display and then new search suggestions.

jimbo · 2020-02-21T16:09:03Z

@jimbo Looks good but still there is minor issue. Input dress wait for search suggestion to show, now clear the search field and enter Top > For fraction of seconds old search suggestions display and then new search suggestions.

That's fine. We don't have any defined loading behavior yet, so it's going to show stale results while the next query is resolving.

Fix suggestions bug

bb08405

jimbo added the version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump. label Feb 7, 2020

Revert navigation

09e2aa5

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Feb 7, 2020

Fix tests

143f3de

sirugh changed the title ~~Hide stale suggestions~~ [bugfix] Hide stale suggestions Feb 7, 2020

Merge branch 'develop' into jimbo/talon-demo

5b6aa61

tjwiebell suggested changes Feb 11, 2020

View reviewed changes

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js Outdated Show resolved Hide resolved

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js Outdated Show resolved Hide resolved

packages/peregrine/lib/talons/SearchBar/useSearchBar.js Outdated Show resolved Hide resolved

m2-community-project bot assigned tjwiebell Feb 11, 2020

m2-community-project bot added the Progress: changes requested label Feb 11, 2020

Address PR feedback

1fefe7a

m2-community-project bot assigned jimbo Feb 11, 2020

m2-community-project bot added Progress: review and removed Progress: changes requested labels Feb 11, 2020

jimbo and others added 2 commits February 12, 2020 16:54

Fix tests

5e5532f

Remove state update from effect

873c817

tjwiebell previously approved these changes Feb 13, 2020

View reviewed changes

Merge branch 'develop' into jimbo/talon-demo

4db7b4d

Merge branch 'develop' into jimbo/talon-demo

1dd257d

sirugh reviewed Feb 19, 2020

View reviewed changes

m2-community-project bot assigned sirugh Feb 19, 2020

Only search if valid

2186a78

Signed-off-by: Stephen Rugh <[email protected]>

sirugh dismissed tjwiebell’s stale review via 2186a78 February 19, 2020 17:46

Merge branch 'develop' into jimbo/talon-demo

a1d7131

dpatil-magento previously approved these changes Feb 19, 2020

View reviewed changes

m2-community-project bot assigned dpatil-magento Feb 19, 2020

Change state from cleared to valid

1bb87b4

jimbo dismissed dpatil-magento’s stale review via 1bb87b4 February 19, 2020 21:56

Merge branch 'develop' into jimbo/talon-demo

1709338

Stop writing param to search field

1868775

sirugh reviewed Feb 20, 2020

View reviewed changes

packages/peregrine/lib/talons/SearchBar/useAutocomplete.js Outdated Show resolved Hide resolved

jimbo added 2 commits February 20, 2020 14:21

Remove logs

7bf99a3

Fix tests

bf25043

sirugh reviewed Feb 20, 2020

View reviewed changes

sirugh approved these changes Feb 20, 2020

View reviewed changes

Merge branch 'develop' into jimbo/talon-demo

9f8b0b2

Merge branch 'develop' into jimbo/talon-demo

229aa40

supernova-at merged commit 6d9df6e into develop Feb 21, 2020

supernova-at deleted the jimbo/talon-demo branch February 21, 2020 17:12

m2-community-project bot removed the Progress: review label Feb 21, 2020

sirugh mentioned this pull request Feb 24, 2020

[feature]: Connect Filtering to GraphQL! #2166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bugfix] Hide stale suggestions #2150

[bugfix] Hide stale suggestions #2150

jimbo commented Feb 7, 2020 •

edited by sirugh

Loading

PWAStudioBot commented Feb 7, 2020 •

edited

Loading

tjwiebell left a comment

tjwiebell left a comment

dpatil-magento commented Feb 13, 2020 •

edited

Loading

tjwiebell commented Feb 13, 2020 •

edited

Loading

jimbo commented Feb 14, 2020

sirugh Feb 19, 2020

sirugh Feb 19, 2020

jimbo commented Feb 19, 2020

dpatil-magento commented Feb 19, 2020

dpatil-magento commented Feb 20, 2020

jimbo commented Feb 20, 2020

sirugh Feb 20, 2020 •

edited

Loading

sirugh left a comment

dpatil-magento commented Feb 21, 2020 •

edited

Loading

jimbo commented Feb 21, 2020

[bugfix] Hide stale suggestions #2150

[bugfix] Hide stale suggestions #2150

Conversation

jimbo commented Feb 7, 2020 • edited by sirugh Loading

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Feb 7, 2020 • edited Loading

tjwiebell left a comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

dpatil-magento commented Feb 13, 2020 • edited Loading

tjwiebell commented Feb 13, 2020 • edited Loading

jimbo commented Feb 14, 2020

sirugh Feb 19, 2020

Choose a reason for hiding this comment

sirugh Feb 19, 2020

Choose a reason for hiding this comment

jimbo commented Feb 19, 2020

dpatil-magento commented Feb 19, 2020

dpatil-magento commented Feb 20, 2020

jimbo commented Feb 20, 2020

sirugh Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

sirugh left a comment

Choose a reason for hiding this comment

dpatil-magento commented Feb 21, 2020 • edited Loading

jimbo commented Feb 21, 2020

jimbo commented Feb 7, 2020 •

edited by sirugh

Loading

PWAStudioBot commented Feb 7, 2020 •

edited

Loading

dpatil-magento commented Feb 13, 2020 •

edited

Loading

tjwiebell commented Feb 13, 2020 •

edited

Loading

sirugh Feb 20, 2020 •

edited

Loading

dpatil-magento commented Feb 21, 2020 •

edited

Loading