Improve library loading UX#7119
Conversation
| } | ||
|
|
||
| public void onDatabaseLoadingFailed(LibraryTab libraryTab, Exception ex) { | ||
| dialogService.showErrorDialogAndWait(Localization.lang("Connection error"), |
There was a problem hiding this comment.
It would be cook if you could pass the exception as paramter to the error dialog as well, it's just one addtional paramter
|
Wohoo! Thanks for your work, I just tested it with a huge library, it's really cool and looks really nice! |
|
I'll take a look into it the next days, im just a bit busy this week, I hope you don't mind. |
Don't worry I'm not in a hurry, take your time |
tobiasdiez
left a comment
There was a problem hiding this comment.
Thanks also from my side. Some initial suggestions:
| } | ||
|
|
||
| /* The layout to display in the tab when it's loading*/ | ||
| public Node createLoadingLayout() { |
There was a problem hiding this comment.
I would let the LibraryTab handle the progress indicator. This could be done for example by setting the placeholder to the progress indicator. https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/TableView.html#placeholderProperty
There was a problem hiding this comment.
There was a bug when I close a loading tab before it finishes, the tab next to it TableView would shrink but it seems using placeholder fixed it, Thank you
| BibDatabaseContext context = result.getDatabaseContext(); | ||
| libraryTab.feedData(context); | ||
|
|
||
| OpenDatabaseAction.performPostOpenActions(libraryTab, result); |
There was a problem hiding this comment.
I would put this before the feedData call. Some of the post actions change a lot of entries, which would result in unnecessary updates of the ui.
| context.getDatabasePath().isPresent(); | ||
| } | ||
|
|
||
| private void trackOpenNewDatabase(LibraryTab libraryTab) { |
There was a problem hiding this comment.
I think this should be in the opendatabaseaction class (as it's not really connected to the library tab)
There was a problem hiding this comment.
and by this, you mean trackOpenNewDatabase(), right?
| } | ||
|
|
||
| public BackgroundTask<?> getDataLoadingTask() { | ||
| if (dataLoadingTask == null) { |
There was a problem hiding this comment.
This condition/if is uncessary as you either return null or the dataLoadingTask.
DominikVoigt
left a comment
There was a problem hiding this comment.
Great work!
I did not test the changes, but the code looks fine to me :)!
|
I will fix some line breaks and merge tonight. Sorry for delay. |
|
Thanks again for your contribution! |
* upstream/master: Improve library loading UX (#7119) remove jython (#7157) Add missing authors Remove obsolete hint Fixed issue in PreviewView for number textfield (#7158) Fix for issue 6959 (#7151) Revert "remove jython (#7155)" (#7156) remove jython (#7155) Fix remembering password for sql db (#7154) Update to libre office 7.0.3 (#7150) Add IdBasedSearchFetcher to jstor (#7145) Squashed 'src/main/resources/csl-styles/' changes from 55200d0..a20406d Bump antlr4-runtime from 4.8-1 to 4.9 (#7136)
When JabRef opens a library, it opens a file, runs the database parser, and after everything is done, creates a new tab in the frame with the contents of the open database.
An improvement would be to first create a tab in the frame, display a loading animation, and after the parser has finished, display all entries (or display every entry as soon as it is parsed).
there is a bug that I couldn't fix, even after data is loaded the group pane will keep showing 0 entry, you need to navigate away from the tab and back to get the real number of entries
about applying the improvement on startup, I think having a splash screen will look better something like a blocking dialog that displays a ProgressIndicator
Closes #6417
