Skip to content

Conversation

g4rry420
Copy link
Contributor

Signed-off-by: GurkiranSingh gurkiransinghk@gmail.com

What does this PR do?

Fixes the navigation to searchField in Select component using up arrow key

Where should the reviewer start?

SelectContainer.js

What testing has been done on this PR?

N/A

How should this be manually tested?

Using the storybook Search Example, try navigating down and up the SearchField in Select component

Do Jest tests follow these best practices?

N/A

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

N/A

What are the relevant issues?

#6096

Screenshots (if appropriate)

N/A

Do the grommet docs need to be updated?

No

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

backwards compatible

Signed-off-by: GurkiranSingh <gurkiransinghk@gmail.com>
Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

When I navigate back up to the search field and hit 'space' the first option is being selected instead of a 'space' being entered into the search

Signed-off-by: GurkiranSingh <gurkiransinghk@gmail.com>
@g4rry420
Copy link
Contributor Author

@jcfilben Thanks for pointing out that issue. I have fixed it. Please, let me know if you see any issues :)

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@jcfilben jcfilben requested a review from ericsoderberghp May 2, 2022 23:06
@ericsoderberghp ericsoderberghp merged commit 7742bc1 into grommet:master May 3, 2022
@g4rry420 g4rry420 deleted the issue_6096 branch October 28, 2022 01:01
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.

3 participants