Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix#8839: Cannot select item from command selector #8842

Merged
merged 3 commits into from Aug 17, 2023

Conversation

Clem-Fern
Copy link
Contributor

@Clem-Fern Clem-Fern commented Aug 17, 2023

Hi @Eugeny, I hope all is well with you.

Fix #8839 : This PR aims to fix the issue #8839. Commit d354520 avoid the selectedIndex value to be NaN cause of modulo (this.selectedIndex + this.filteredOptions.length) % this.filteredOptions.length when filteredOptions is empty. Once selectedIndex in NaN, the OnFilterChange method is no longer able to do operation with selectedIndex which cause the selector to do not have any element selected any more even if the filter is empty or matches elements.

ad3b03c: I also added a small feature on PageUp & PageDown event to limit the number of elements being skipped when the key is press.

if (event.key === 'PageUp' || event.key === 'ArrowUp' && event.metaKey) {
this.selectedIndex -= Math.min(10, Math.max(1, this.selectedIndex))
event.preventDefault()
} else if (event.key === 'PageDown' || event.key === 'ArrowDown' && event.metaKey) {
this.selectedIndex += Math.min(10, Math.max(1, this.filteredOptions.length - this.selectedIndex - 1))
event.preventDefault()

On 1.0.197:
Open selector. Select the second element before the end and press PageDown.
The eighth from the start will be the one selected now.

With this PR:
Open selector. Select the second element before the end and press PageDown.
The last element of the list will be the one selected now.
Press PageDown again.
It will select the first element from the top of the list.
Same inverse behavior on PageUp.

As always, feel free to ask me if any change are needed !

@Clem-Fern Clem-Fern changed the title Fix#8839 Fix#8839: Cannot select item from command selector Aug 17, 2023
@Eugeny
Copy link
Owner

Eugeny commented Aug 17, 2023

LGTM, great job!

@Eugeny Eugeny merged commit 34786b1 into Eugeny:master Aug 17, 2023
10 checks passed
@Clem-Fern Clem-Fern deleted the fix#8839 branch September 2, 2023 14:47
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.

Cannot select item from command selector
2 participants