Skip to content
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.

Sign up for GitHub

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

Add change listener to main table to scroll to imported entry #5421

Merged
merged 10 commits into from
Oct 16, 2019
Merged

Add change listener to main table to scroll to imported entry #5421

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cae560e
Add change listener to main table to scroll to imported entry #5383
b0n541 Oct 9, 2019
bb1b11d
Add fix to CHANGELOG.md #5383
b0n541 Oct 10, 2019
12b943d
Introduce new event for finished insertion of one or multiple entries
b0n541 Oct 14, 2019
f233a91
Make CHANGELOG entry more user friendly #5383
b0n541 Oct 14, 2019
9f53fd5
Fix checkstyle issues
b0n541 Oct 14, 2019
4f5ebc6
Merge remote-tracking branch 'upstream/master' into scroll_to_importe…
b0n541 Oct 14, 2019
983262c
Restore posting of entry added event for each new bib entry
b0n541 Oct 14, 2019
7d41097
Merge remote-tracking branch 'upstream/master' into scroll_to_importe…
b0n541 Oct 15, 2019
c17d202
White space change to trigger pull request build
b0n541 Oct 15, 2019
aa48000
Merge remote-tracking branch 'upstream/master' into scroll_to_importe…
b0n541 Oct 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed a problem where the "editor" information has been duplicated during saving a .bib-Database. [#5359](https://github.com/JabRef/jabref/issues/5359)
- We re-introduced the feature to switch between different preview styles. [#5221](https://github.com/JabRef/jabref/issues/5221)
- We fixed various issues (including [#5263](https://github.com/JabRef/jabref/issues/5263)) related to copying entries to the clipboard

- We added a change listener to the main table to scroll to imported entry [#5383](https://github.com/JabRef/jabref/issues/5383)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change log is mostly for our users and most of them don't know what a "change listener" is ;-). Please reformulate this a bit more user-friendly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.




Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/jabref/gui/maintable/MainTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.util.ControlHelper;
import org.jabref.gui.util.CustomLocalDragboard;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.ViewModelTableRowFactory;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.UpdateField;
Expand Down Expand Up @@ -112,6 +113,15 @@ public MainTable(MainTableDataModel model, JabRefFrame frame,
// Enable sorting
model.getEntriesFilteredAndSorted().comparatorProperty().bind(this.comparatorProperty());

model.getEntriesFilteredAndSorted().addListener(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it is a neat idea to use the listener, I'm afraid that it does not work as intended. The listener is also triggered if I clear a search as then a bunch of previously filtered-out entries are readded to the list. What should work is if you add a listener to database.getDatabase().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into multiple race conditions after I added a change listener to the internal observable list of bib entries in the database. Therefore I choose to introduce a new event that is sent over the event bus when the insertion of one or many entries is finished.

(ListChangeListener<BibEntryTableViewModel>) change -> {
if (change.next()) {
if (change.wasAdded()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also add a !change.getAddedSubList().isEmpty() check to make sure that the code below executes without exception (that test might be covered by wasAdded but I'm not sure)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's covered by wasAdded().

DefaultTaskExecutor.runInJavaFXThread(() -> clearAndSelect(change.getAddedSubList().get(0).getEntry()));
}
}
});

this.panel = panel;

pane = new ScrollPane(this);
Expand Down