Fix Preferences Search #6946
Fix Preferences Search #6946
Conversation
updated fxml file to be able to appear in preference search.
|
Hi @fabiojavamarcos , I think a different approach would be to design a tree walker in |
calixtus
added
the
status: changes required
|
Agree with @calixtus , a simple recursive appraoch https://stackoverflow.com/a/13011616 should work |
|
Ok. I am going to take a look! |
Created a recursive method to find all child node from each tab in preferences search
|
Now I think it is better! Thanks for the suggestion. |
|
With this recursive approach you should be able to put all the Labeled nodes in the fxml files back into the hboxes. If you like to look a bit around in the JabRef sources, you could also think of moving this recursive search init stuff into a background task, if the preferences dialog takes too long to load now, and grey the search box out as long as the preferences are indexed (probably not much more than a fracture of a second). But maybe this too much for this PR. The most important thing is now that the issue has been fixed. |
|
Ok. Doing that! Here the indexing was fast. But... you know... I am fixing the HBox and you decide if is better to finish or not. Maybe finish it and open another issue can be ok. And I can start looking at the new one as well. Thanks! |
rollback controls inside HBoxes and changing to static scanInputControls
| @@ -89,7 +89,7 @@ private void clearSearch() { | |||
| return filteredPreferenceTabs; | |||
| } | |||
|
|
|||
| private void scanInputControls(Parent parent, ArrayListMultimap<PreferencesTab, Labeled> prefsTabLabelMap, PreferencesTab preferencesTab) { | |||
| private static void scanInputControls(Parent parent, ArrayListMultimap<PreferencesTab, Labeled> prefsTabLabelMap, PreferencesTab preferencesTab) { | |||
There was a problem hiding this comment.
Please give ths method a more meaningful name. You are scanning for label controls.
method name updated to: scanLabeledControls
| @@ -24,7 +24,7 @@ | |||
| <CheckBox fx:id="displayGroupCount" text="%Display count of items in group"/> | |||
|
|
|||
| <HBox spacing="4" alignment="CENTER_LEFT"> | |||
| <Label text="%Keyword separator"/> | |||
| <Label text="%Keyword seperator"/> | |||
fix typo
| <?import javafx.scene.layout.VBox?> | ||
| <?import org.jabref.gui.commonfxcontrols.SaveOrderConfigPanel?> | ||
| <?import javafx.scene.control.ToggleGroup?> |
There was a problem hiding this comment.
Is there any system in reordering the imports in the fxmls? If not, why not leave it alphabetically like every other import in the .Java files...
ordering javafx imports
There was a problem hiding this comment.
Im afraid the code style issues are still remaining. See:
https://github.com/JabRef/jabref/pull/6946/files
But don't mind, we can fix this before merging it.
Changes to PreferencesSearchHandler look good to me.
|
I have no idea why they changed their positions because I did not change them! Anyway, I checked again and manually put it in alphabetical order. |
There was a problem hiding this comment.
Looks very good to me. Since @Siedlerchr already approved it, I think I'm going to merge it.
Thank you very much for this PR. We are happy to welcome you to the contributors of JabRef. Even if our remarks sometimes sound a bit lit nitpicking, it's because we try to lift the code style standards in JabRef for some months now. But open source is always about learning something new and working together for something good. We hope you liked working on JabRef an you consider to return for another issue to fix or feature to implement!
So thanks again!
|
I am really having fun, learning with the process, with your experience, and happy! I am going to check more issues. Thanks for your orientation and patience! |
Updated fxml files from the preferences package to appear in preference search.
The preferences search was unable to find some visual components because they are not a direct child from one of the preferences tab.
So, the method getPrefsTabLabelMap in PreferencesSearchHandler cannot find them. I moved some components that were inside tags to let them "visible" to their parents in the mentioned method. It does not change the UI. Most of the components could be moved. Some could not because they have many parents until the tab and it could impact the UI.
Fixes #6866