-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix: changed preferences and keybind search #12739
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
Conversation
…trols like combo boxes
| boolean controlMatches = preferenceTabsControls.get(tab).stream() | ||
| .filter(control -> controlMatchesQuery(control, query)) | ||
| .peek(this::highlightControl) | ||
| .findAny() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using findAny().isPresent() is less readable than anyMatch(). The stream operation could be simplified to improve code clarity and follow modern Java practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont see how something like this would be more readable:
boolean controlMatches = preferenceTabsControls.get(tab).stream().anyMatch(control -> {
boolean matches = controlMatchesQuery(control, query);
if (matches) {
highlightControl(control);
}
return matches;
});
1e35633 to
d1ffbed
Compare
|
Force push was because I forgot to put the changes for the codestyle in KeyBindingsTabViewModel in gitbutler in the right branch. So I put it in the corresponding commit afterwards and had to force push. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request improves the search functionality within the preferences view and keybindings configuration. It enables searching based on values in comboboxes, text fields, and key combinations, while adding an automatic expand feature for matching keybinding categories alongside "Expand all"/"Collapse all" buttons.
- Preferences search now inspects additional control types (comboboxes and text fields) for matching the query.
- Keybindings search has been enhanced to support searching by key combinations and to auto-expand matching categories.
- Added UI controls for expanding/collapsing all keybinding categories and updated documentation in the CHANGELOG.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/org/jabref/gui/preferences/PreferencesSearchHandler.java | Refactored search filtering and highlighting for additional control types. |
| src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTabViewModel.java | Enhanced keybindings search logic and introduced a dedicated query matching method. |
| src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java | Updated search listener and added expand/collapse functionality. |
| CHANGELOG.md | Updated release notes reflecting the new search features. |
Comments suppressed due to low confidence (2)
src/main/java/org/jabref/gui/preferences/PreferencesSearchHandler.java:98
- The new search functionality that matches combobox and textfield content is critical; consider adding tests to cover various control types and scenarios to ensure consistent behavior.
private boolean controlMatchesQuery(Control control, String query) {
src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTabViewModel.java:89
- Since the matchesSearchTerm method plays a key role in the updated keybindings search, adding tests that cover localization matches, category matches, and key combination matches is recommended.
private boolean matchesSearchTerm(KeyBinding keyBinding, String searchTerm) {
koppor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides minor comments, LGTM 👍. The changes need a second review and thus this PR awaits a second review.
src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
about wrong intellij autoformat and nullpointer check added due to trag bot complaining about it
src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
Show resolved
Hide resolved
koppor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out, works well.
…2739) * changed label only index- and searching to also account for other controls like combo boxes * Merge branch 'JabRef:main' into fix-issue-12647 * add textfield search * enhance keybindings search in preferences * fix styling and names * added the changes made to CHANGELOG.md * fix trag bot comment for potential nullpointer * fix codestyle * fix trag bots comment about the javadocs missing explanation for the logic * fix openrewrite test failing * Update CHANGELOG.md Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> * fixed comments from @koppor about wrong intellij autoformat and nullpointer check added due to trag bot complaining about it --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
I've improved the search functionality in the preferences view to include values from comboboxes and text fields, not just labeled controls. Previously, searching for "VScode" returned no results, requiring users to search for "push" instead. Now, comboboxes or textfields containing the searched term can be found directly.
In addition, when searching keybindings, matching categories now expand automatically, which wasn't the case before. I've also added "Expand all" and "Collapse all" buttons at the bottom of the tree table for easier navigation.
It's also now possible to search directly for specific key combinations (e.g., "ctrl+w") to quickly locate the associated actions.
Closes #12647
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)