Merged
Conversation
Passing run #1751 ↗︎
Details:
Review all test suite changes for PR #786 ↗︎ |
|||||||||||||||
LukasJoss
approved these changes
Apr 15, 2024
| @focus="onSearchInputFocus" | ||
| @keydown.down.prevent="goToFirstResult" | ||
| @keydown.esc.prevent="closeSearchResults" | ||
| @keydown.esc.prevent="toggleResults" |
Contributor
There was a problem hiding this comment.
The escape toggle is only working when focus is on the search bar and not when it is focused on a search result.
Contributor
Author
There was a problem hiding this comment.
Yes I know but this would make the code a bit more complicated as we would need to pass the event up a few hierarchy and we would also need to correctly manage focus.
Contributor
Author
There was a problem hiding this comment.
@LukasJoss Actually on old viewer the ESC was clearing the input which make more sense what do you think ?
Contributor
There was a problem hiding this comment.
I prefer the toggle but its up to you
Contributor
Author
There was a problem hiding this comment.
Ok let be the toggle, chatgpt answer on that
In keyboard navigation in an input text field what is the most common action triggered by ESC ?
ChatGPT
In keyboard navigation within an input text field, pressing the ESC key commonly triggers the action of canceling or closing the current operation or modal window associated with the input field.
For example:
If the input field is part of a search bar or a dropdown menu, pressing ESC might close the dropdown or cancel the search operation.
If the input field is within a modal window, pressing ESC might close the modal and discard any changes made in the input field.
Overall, the ESC key is often used as a convenient way for users to dismiss or cancel ongoing actions or interactions related to input fields.
When entering in the search bar regex special characters the search bar could crash especially when entering `(` or `)`.
When closing the search bar result we focus on the input in order to be able to directly change the input again. When selecting a result entry we then close the results but also focus on the search input. Now the ESC key is a toggling result and when pressing enter on the search input we jump to the first result (opening the result if needed)
During e2e tests of the search bar, the console had errors due to invalid search layer mocking response.
When removing the swisssearch from the url or when removing its value, a null value was set in the store which make a crash (happened during e2e tests). Now we are using the parameter default value
The layer parameter in this case is a string and not an object
3c8b59b to
7e07344
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When clicking on a search bar result make sure to set the focus back on the search bar, so the user can quickly edit the search if needed.
Test link