fix(atomic): don't redirect when hovering a instant result & pressing Enter #4938
Conversation
alexprudhomme
requested review from
olamothe,
fpbrault,
y-lakhdar and
fbeaudoincoveo
Pull Request ReportPR Title✅ Title follows the conventional commit spec. Live demo linksBundle Size
SSR Progress
Detailed logssearch : buildInteractiveResultsearch : buildInteractiveInstantResult search : buildInteractiveRecentResult search : buildInteractiveCitation search : buildGeneratedAnswer recommendation : missing SSR support case-assist : missing SSR support insight : missing SSR support commerce : missing SSR support |
alexprudhomme
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
|
There's a regression here: we still want to navigate/select the product if the product is focused/selected with keyboard |
alexprudhomme
force-pushed
the
KIT-3910
branch
from
27308a2 to
7957507
Compare
alexprudhomme
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
alexprudhomme
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
| @@ -304,6 +304,17 @@ test.describe('with instant results & query suggestions', () => { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| test.describe('when hovering a instant result and pressing Enter', () => { | |||
| test('should not redirect to the instant product url', async ({ | |||
There was a problem hiding this comment.
Is this really what we're testing here?
The way I understand it, we're clicking in the search intput, hovering the first intant result, pressing enter, and expecting the search input to have an empty value. This would be the case even if a redirection occurred, no?
I think what we actually want to test is that when you hover an instant result and press enter, it executes the query that's currently in the search box.
| test('should be able to navigate from the suggestions to the instant results and vice-versa', async () => {}); | ||
| test('should execute the search after pressing "Enter" on a suggestion', async () => {}); | ||
| test('should redirect to the result page after pressing "Enter" on an instant result', async () => {}); | ||
| describe('and then switching to mouse and hovering a new suggestion', () => { |
There was a problem hiding this comment.
Not sure I would structure the tests this way.
I think I'd have a describe at the same level as "when using keyboard navigation" that would be something like "when switching between keyboard and mouse navigation"
| test('it should execute the search reflected in the search box', () => {}); | ||
| }); | ||
|
|
||
| // This is a bug currently in the component |
There was a problem hiding this comment.
Do we have a Jira to fix it? If not we should create one and mention it here in a TODO
| @@ -0,0 +1,19 @@ | |||
| import {describe, test} from 'vitest'; | |||
|
|
|||
| //TODO: Write those tests during the lit migration | |||
There was a problem hiding this comment.
Mention KIT-3955 here
| @@ -0,0 +1,23 @@ | |||
| import {describe, test} from 'vitest'; | |||
|
|
|||
| //TODO: Write those tests during the lit migration | |||
| @@ -0,0 +1,17 @@ | |||
| import {describe, test} from 'vitest'; | |||
|
|
|||
| //TODO: Write those tests during the lit migration | |||
| @@ -0,0 +1,19 @@ | |||
| import {describe, test} from 'vitest'; | |||
|
|
|||
| //TODO: Write those tests during the lit migration | |||
https://coveord.atlassian.net/browse/KIT-3910
We now have a new variable in suggestions-manager that tracks they keyboard use. When someone press Enter when having a suggestion/result/recent query selected with the keyboard, it will trigger it's onClick event.
Whenever we stop using the keyboard and using the mouse it resets again which will make the Enter event trigger the query in the search box instead.
I added a lot of tests to write in vitest.