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

SearchHistoryMenu: improve scrolling through selection with arrow keys #2232

Merged

Conversation

Wittmaxi
Copy link
Contributor

@Wittmaxi Wittmaxi commented Sep 2, 2024

Scrolling through the selection of the Search History now correctly starts at the first item and cycles through the boundaries (ie. scrolling down from the last item returns correctly to the first item of the list)

fixes #2139

34

Copy link
Contributor

github-actions bot commented Sep 2, 2024

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 43m 59s ⏱️ + 1m 39s
 7 709 tests ±0   7 481 ✅ ±0  228 💤 ±0  0 ❌ ±0 
24 288 runs  ±0  23 541 ✅ ±0  747 💤 ±0  0 ❌ ±0 

Results for commit ad27024. ± Comparison against base commit 6d7fa39.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Wouldn't it also be an option to set the selection to the first (or even to the currently selected, if existing) entry in the table? Then you would always have a selection and do not require this additional functionality replicating the complete navigation functionality.

Note that in usual dropdowns, hovering over other elements also moves the selection to these entries, so that the keyboard navigation will proceed from the entry you have hovered over last.

@Wittmaxi Wittmaxi force-pushed the MW_dropdown_navigate_to_first branch 2 times, most recently from 3a159e7 to 7ad0aad Compare September 10, 2024 16:06
@Wittmaxi
Copy link
Contributor Author

The hover is now sticky such that the navigation with the arrow keys resumes from the position where the last hover happened.
This also meant that some state (the scrol positoin) had to be moved into the entire Search History Menu instead of in the key listener (see comment above).

That way, I could fix the comment about using the adapter for the listener on the fly.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

The change looks okay and works as expected. I only have a nitpicky proposal for improvement.

@Wittmaxi
Copy link
Contributor Author

@HeikoKlare if this is fine with you, I will merge by end of day

@deepika-u
Copy link
Contributor

Verified on the below build applying the pr and i am able to select 1st entry when using down arrow key on Find/replace overlay.

Eclipse SDK
Version: 2024-12 (4.34)
Build id: I20240923-1800
OS: Windows 11, v.10.0, x86_64 / win32
Java vendor: Eclipse Adoptium
Java runtime version: 21.0.1+12-LTS
Java version: 21.0.1

Scrolling through the selection of the Search History now correctly
starts at the first item and cycles through the boundaries (ie.
scrolling down from the last item returns correctly to the first item of
the list)

fixes eclipse-platform#2139
@HeikoKlare HeikoKlare merged commit dea7b79 into eclipse-platform:master Oct 11, 2024
17 checks passed
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.

Find/replace overlay: history drop-down navigates to second entry when pressing "down" key
3 participants