Feature: implement search filter in show preferences #4759

CaptainDaVinci · 2019-03-13T17:41:02Z

Add a search box in show preferences window to allow users to filter
preference options.

Resolves #1019

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Add a search box in show preferences window to allow users to filter preference options. Resolves #1019

Siedlerchr · 2019-03-13T17:53:17Z

src/main/java/org/jabref/gui/preferences/PreferencesFilterDialog.java

+        }
+
+        List<JabRefPreferencesFilter.PreferenceOption> filteredOptions = auxillaryPreferenceOptions.stream()
+                                                                                                   .filter(p -> p.getKey().toLowerCase().contains(searchText))


Better use LOCALE.ROOT as second parameter for the toLowerCase function to avoid unexpected problems:
https://garygregory.wordpress.com/2015/11/03/java-lowercase-conversion-turkey/

Understood, I have made the necessary changes.

Siedlerchr · 2019-03-13T19:25:12Z

Codewise lgtm. Could you please provide a screenshot how it looks like when the search matched/text is found?

tobiasdiez

Thanks for your PR. The code looks good and works, but can still be a bit improved using a FilteredList (which encapsulates some of the things you had to implement your own).

tobiasdiez · 2019-03-13T20:11:24Z

src/main/java/org/jabref/gui/preferences/PreferencesFilterDialog.fxml

@@ -22,6 +23,12 @@
                    <Label fx:id="count"/>
                </HBox>
            </bottom>
+            <top>
+                <HBox>
+                    <TextField fx:id="searchField" promptText="Search"/>


Please use %Search so that the text get localized properly.

tobiasdiez · 2019-03-13T20:15:33Z

src/main/java/org/jabref/gui/preferences/PreferencesFilterDialog.java

@@ -22,6 +26,7 @@

    private final JabRefPreferencesFilter preferencesFilter;
    private final ObservableList<JabRefPreferencesFilter.PreferenceOption> preferenceOptions;
+    private final List<JabRefPreferencesFilter.PreferenceOption> auxillaryPreferenceOptions;


If you use the FilteredList, then the code gets somewhat cleaner (as you don't need to manage two lists on your own). Have a look here:

jabref/src/main/java/org/jabref/gui/maintable/MainTableDataModel.java

Lines 29 to 33 in b39ed7b

entriesFiltered = new FilteredList<>(entriesViewModel);

entriesFiltered.predicateProperty().bind(

Bindings.createObjectBinding(() -> this::isMatched,

Globals.stateManager.activeGroupProperty(), Globals.stateManager.activeSearchQueryProperty())

);

Thank you for the suggestions! I have made the necessary changes.

CaptainDaVinci · 2019-03-14T14:52:38Z

@Siedlerchr I have added a screenshot now.

CaptainDaVinci · 2019-03-14T14:54:09Z

Will moving the checkbox next to the search field provide a better UX?
Should the count label be bound to filteredOptions.size()?

Siedlerchr · 2019-03-14T15:16:26Z

Yeah! That's cool, looks good, I would move the checkbox to the top next to the search field.
Regarding your second option: Either you directly bind it to the filtered list size property or you update the text in the method which is your current way. But both together will result in an error: A bound value cannot be set ;)
So as you automatically call the updateModel method on changes, you can leave it as is.

CaptainDaVinci · 2019-03-15T07:15:16Z

But the updatedModel() is called only when check box is changed, I was wondering if having it update while filtering would be a good addition.

CaptainDaVinci · 2019-03-15T12:54:12Z

On moving the checkbox to the top next to the search field, what are your opinion on it @tobiasdiez @Siedlerchr ?

Siedlerchr · 2019-03-15T13:39:59Z

Yep, that is exactly what I meant'! Looks good! And thanks for fixing this longstanding issue ;)

CaptainDaVinci · 2019-03-15T14:03:26Z

I am glad it came out as expected! :)
However, I think that the count label should be bound to the table size, so that the right count is shown when filtering. I will push the changes and we can decide on whether to merge the change or stick with the current setup.

tobiasdiez

Thanks! The code looks good and I like the idea of having the count bound to the actual number of entries displayed.

…erbibtexkey * upstream/master: (827 commits) [#4306] Disable renaming (#4727) Feature: implement search filter in show preferences (#4759) Enable import button only if entry selected (#4761) Improve Remote messaging (#4760) Add changelog entry for #4520 Modifications to improve contrast of UI elements (#4758) Rework external changes dialog in JavaFX (#4693) Changes the title of Group Dialog to Add subgroup when adding a new subgroup (#4757) Optimize data fetching (#4520) Adds a browse button next to the path text field for aux-based groups (#4743) Saving changes and exiting application (#4729) Code optimized and created a reusable method to get writer output (#4750) Bump mvvmfx from 1.7.0 to 1.8.0 (#4747) Bump guava from 27.0.1-jre to 27.1-jre (#4748) Bump mvvmfx-validation from 1.7.0 to 1.8.0 (#4749) Remove obsolete swing components in Preferences and OpenOffice (#4740) Move library specific key pattern dialog call to library menu (#4744) Revert "Revert "Fix: bibkey generated does not handle diacritics" (#4741)" (#4742) Revert "Fix: bibkey generated does not handle diacritics" (#4741) Rename method to removeUnwantedCharacters ...

Feature: implement search filter in show preferences

74048b6

Add a search box in show preferences window to allow users to filter preference options. Resolves #1019

Siedlerchr reviewed Mar 13, 2019

View reviewed changes

CaptainDaVinci added 3 commits March 13, 2019 23:28

Add Locale.ROOT when converting to lower case

897c612

Fixed indentation for fxml

e377b73

Added Locale.ROOT when converting to lower case

d7e1f59

Siedlerchr approved these changes Mar 13, 2019

View reviewed changes

tobiasdiez requested changes Mar 13, 2019

View reviewed changes

Using FilteredList to filter options on search

9d9d172

set the count label text, which was accidently removed

259626c

CaptainDaVinci added 2 commits March 15, 2019 19:11

Moved checkbox to the top

a7a7e79

Bind count label to table size property

882aa6e

tobiasdiez approved these changes Mar 15, 2019

View reviewed changes

tobiasdiez merged commit cd979c5 into JabRef:master Mar 15, 2019

WtfJoke mentioned this pull request Mar 16, 2019

Make Show Preferences Window searchable #1019

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: implement search filter in show preferences #4759

Feature: implement search filter in show preferences #4759

CaptainDaVinci commented Mar 13, 2019 •

edited

Loading

Siedlerchr Mar 13, 2019

CaptainDaVinci Mar 13, 2019

Siedlerchr commented Mar 13, 2019

tobiasdiez left a comment

tobiasdiez Mar 13, 2019

tobiasdiez Mar 13, 2019

CaptainDaVinci Mar 14, 2019

CaptainDaVinci commented Mar 14, 2019

CaptainDaVinci commented Mar 14, 2019 •

edited

Loading

Siedlerchr commented Mar 14, 2019

CaptainDaVinci commented Mar 15, 2019

CaptainDaVinci commented Mar 15, 2019 •

edited

Loading

Siedlerchr commented Mar 15, 2019

CaptainDaVinci commented Mar 15, 2019 •

edited

Loading

tobiasdiez left a comment

	entriesFiltered = new FilteredList<>(entriesViewModel);
	entriesFiltered.predicateProperty().bind(
	Bindings.createObjectBinding(() -> this::isMatched,
	Globals.stateManager.activeGroupProperty(), Globals.stateManager.activeSearchQueryProperty())
	);

Feature: implement search filter in show preferences #4759

Feature: implement search filter in show preferences #4759

Conversation

CaptainDaVinci commented Mar 13, 2019 • edited Loading

Siedlerchr Mar 13, 2019

Choose a reason for hiding this comment

CaptainDaVinci Mar 13, 2019

Choose a reason for hiding this comment

Siedlerchr commented Mar 13, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Mar 13, 2019

Choose a reason for hiding this comment

tobiasdiez Mar 13, 2019

Choose a reason for hiding this comment

CaptainDaVinci Mar 14, 2019

Choose a reason for hiding this comment

CaptainDaVinci commented Mar 14, 2019

CaptainDaVinci commented Mar 14, 2019 • edited Loading

Siedlerchr commented Mar 14, 2019

CaptainDaVinci commented Mar 15, 2019

CaptainDaVinci commented Mar 15, 2019 • edited Loading

Siedlerchr commented Mar 15, 2019

CaptainDaVinci commented Mar 15, 2019 • edited Loading

tobiasdiez left a comment

Choose a reason for hiding this comment

CaptainDaVinci commented Mar 13, 2019 •

edited

Loading

CaptainDaVinci commented Mar 14, 2019 •

edited

Loading

CaptainDaVinci commented Mar 15, 2019 •

edited

Loading

CaptainDaVinci commented Mar 15, 2019 •

edited

Loading