Rework search bar #6682
Rework search bar #6682
Conversation
# Conflicts: # src/main/java/org/jabref/preferences/PreferencesService.java
| @@ -325,7 +322,7 @@ private void updateResults(int matched, TextFlow description, boolean grammarBas | |||
| } | |||
|
|
|||
| private void setHintTooltip(TextFlow description) { | |||
| if (Globals.prefs.getBoolean(JabRefPreferences.SHOW_ADVANCED_HINTS)) { | |||
| if (preferencesService.getGeneralPreferences().shouldShowAdvancedHints()) { | |||
There was a problem hiding this comment.
I think it would make sense to add it to the search preferences as well
There was a problem hiding this comment.
Originally the idea about the advanced hints options was to make it reusable for other hints as well. But in fact, it has only been used also for another (somewhat silly) hint in the network tab in the preferences dialog. So im not sure about this... Maybe in the future, we could evolve that feature to some annoying tip of the day feature? 😉
There was a problem hiding this comment.
I have no strong opinion on this. So then just leave it as is
calixtus
added
the
status: ready-for-review
There was a problem hiding this comment.
The changes look good to me. The only concern that I have is that the search bar is now really small. Thus, while editing a more complex query, it is no longer possible to see the complete query (and "scrolling" in a text field is rather tedious). This was the main reason for the "extend on focus" mode. I'm not particularly attached to the extend animation but it should be still possible to comfortably edit more complex queries.
| public int getSeachDialogWidth() { | ||
| return preferences.getInt(SEARCH_DIALOG_WIDTH); | ||
| } | ||
| public static class Builder { |
There was a problem hiding this comment.
Is the builder really necessary? I think the with* methods can be added directly to the main class without any problems.
There was a problem hiding this comment.
I'll change it, I originally just wanted to stick close to the builder pattern, as I don't have much experience on it, so this was somewhat a study project for me 😉
I'm not quite sure I understand what you mean about the small search bar... I removed the previous restriction of the searchbar to expand to the right, so now it's always extended. The empty space still showing between the searchbar and the toolbar buttons is left over from before and shows "Found X entries". I did not yet had a good idea what to do with this message, so i did not touch it (except adding 4 pixels distance to the searchbar). For comparison: This is how the master branch looks right now: |
|
Ah ok, then I misread the code and the first screenshot - I thought that's now the size of the search bar. |
| searchDisplayMode = SearchDisplayMode.valueOf(get(SEARCH_DISPLAY_MODE)); | ||
| } catch (IllegalArgumentException ex) { | ||
| // Should only occur when the searchmode is set directly via preferences.put and the enum was not used | ||
| searchDisplayMode = SearchDisplayMode.valueOf((String) defaults.get(SEARCH_DISPLAY_MODE)); |
There was a problem hiding this comment.
The get() Method already returns the default if no value:
public String get(String key) {
return prefs.get(key, (String) defaults.get(key));
}
There was a problem hiding this comment.
Good catch. I just copied the logic from the former searchpreferences.class. Fixing the missing consistency is one of the big projects in the future...
There was a problem hiding this comment.
This was the only issue I found so far,the rest looks good!
There was a problem hiding this comment.
Hmm... I thought about this: what if the returned stored value is defect, but not the default value?
There was a problem hiding this comment.
Why should that happen? the Prefernces.get says:
the value associated with key, or def if no value is associated with key, or the backingstore is inaccessible.
|
Maybe the stored value is corrupt and not parseable by value of... |
fixes #6625