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 referenced entry's observers to update the entry in the main-table #7754

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add referenced entry's observers to update the entry in the main-table #7754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0468fa8
fix for issue 7730
SuXiChangZhen May 19, 2021
d0a522a
fix checkstyle
SuXiChangZhen May 19, 2021
abcdcad
Fix caching
k3KAW8Pnf7mkmdSMPHz27 Jul 22, 2021
1b50be6
Remove wrapper for `getReferencedEntry`
k3KAW8Pnf7mkmdSMPHz27 Jul 22, 2021
d14b099
Merge branch 'main' into fix-for-issue-7730
Siedlerchr May 12, 2022
3f6ba17
Merge branch 'main' into fix-for-issue-7730
k3KAW8Pnf7mkmdSMPHz27 May 2, 2023
a58e5d3
Merge remote-tracking branch 'SuXiChangZhen/fix-for-issue-7730' into …
k3KAW8Pnf7mkmdSMPHz27 May 2, 2023
d04621a
Merge branch 'main' into fix-for-issue-7730
koppor Jan 2, 2024
d2242d0
Merge branch 'main' into fix-for-issue-7730
Siedlerchr Dec 9, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where opening the menu 'Library properties' marked the library as modified [#6451](https://github.com/JabRef/jabref/issues/6451)
- We fixed an issue when importing resulted in an exception [#7343](https://github.com/JabRef/jabref/issues/7343)
- We fixed an issue where the field in the Field formatter dropdown selection were sorted in random order. [#7710](https://github.com/JabRef/jabref/issues/7710)
- We fixed an issue where the crossref entry's table preview does not update immediately. [#7730](https://github.com/JabRef/jabref/issues/7730)

### Removed

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/gui/maintable/BibEntryTableViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class BibEntryTableViewModel {
private final BooleanProperty isVisibleByGroup = new SimpleBooleanProperty(true);
private final ObjectProperty<MatchCategory> matchCategory = new SimpleObjectProperty<>(MatchCategory.MATCHING_SEARCH_AND_GROUPS);


public BibEntryTableViewModel(BibEntry entry, BibDatabaseContext bibDatabaseContext, ObservableValue<MainTableFieldValueFormatter> fieldValueFormatter) {
this.entry = entry;
this.bibDatabaseContext = bibDatabaseContext;
Expand Down Expand Up @@ -143,6 +144,8 @@ public ObservableValue<String> getFields(OrFields fields) {
}

ArrayList<Observable> observables = new ArrayList<>(List.of(entry.getObservables()));
Optional<BibEntry> referenced = bibDatabaseContext.getDatabase().getReferencedEntry(entry);
referenced.ifPresent(bibEntry -> observables.addAll(List.of(bibEntry.getObservables())));
Copy link
Member

@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 May 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know enough of JavaFX to tell you for sure how to address this. I'd suggest not removing the caching (fieldValues.get(fields);) and instead deal with the dependencies differently.

  • You can extend StringBinding and use .bind and .unbind to "manually" deal with the dependencies. (this is probably not a good approach)
  • You can extend a different binding (ObjectBinding?) and have it handle changes to the referenced entry. In this scenario, the StringBinding has this ObjectBinding among its dependencies instead of the referenced bibentry's getObservables().
  • At least superficially this seems like a good application for EasyBind because,
    1. You can create an optional binding, e.g., entry.getFieldBinding(StandardField.CROSSREF)
      1. This means you can create an OptionalBinding<BibEntry> referencedBibEntry
    2. It should be possible to keep an observableArrayList with extractor (FXCollections.observableArrayList(BibEntry::getObservables)) updated with the ifValuePresent method of the OptionalBinding<BibEntry> EasyBind.subscribe. That should keep the list up-to-date when the referenced BibEntry is changed/removed and when the observables in the referenced bibEntry.getObservables are modified
    3. This list should now be usable as a dependency

Note that the two last approaches should be used or called from the constructor since they will update on changes to the referenced BibEntry.

I'd appreciate it if someone with more experience of JavaFX chimes in with opinions 😛

Copy link
Member

Choose a reason for hiding this comment

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

Discussion:

@artcile{a,
crossref={b}
}

@book{b}

to

@artcile{a,
crossref={c}
}

@book{b}

@book{c}

Changes to c are not reflected accordingly.

observables.add(fieldValueFormatter);

value = Bindings.createStringBinding(() ->
Expand Down
Loading