From ee300d1acb00d416da321ab531d68c6564dfdd12 Mon Sep 17 00:00:00 2001 From: David Gerber Date: Sat, 16 Nov 2024 21:44:57 +0100 Subject: [PATCH] Refactor the list refresh system Trying to work around the JavaFX bugs regarding tree list updating. Also fixes the trust setting for profile which wasn't set properly. --- .../contact/ContactViewController.java | 104 ++++++++++++------ 1 file changed, 70 insertions(+), 34 deletions(-) diff --git a/ui/src/main/java/io/xeres/ui/controller/contact/ContactViewController.java b/ui/src/main/java/io/xeres/ui/controller/contact/ContactViewController.java index 679c4e15..f4f5bd5a 100644 --- a/ui/src/main/java/io/xeres/ui/controller/contact/ContactViewController.java +++ b/ui/src/main/java/io/xeres/ui/controller/contact/ContactViewController.java @@ -249,7 +249,7 @@ private enum Information // Workaround for https://bugs.openjdk.org/browse/JDK-8090563 private TreeItem selectedItem; private TreeItem displayedContact; - private Contact addedContact; + private boolean contactListLocked; public ContactViewController(ContactClient contactClient, GeneralClient generalClient, ProfileClient profileClient, IdentityClient identityClient, NotificationClient notificationClient, PreferenceService preferenceService, ImageCache imageCacheService, ResourceBundle bundle, WindowManager windowManager, ConfigClient configClient, ConnectionClient connectionClient) { @@ -373,8 +373,6 @@ private void setupContactSearch() }); } - private boolean initiatedByListChange; - private void setupContactTreeTableView() { contactTreeTableNameColumn.setCellFactory(param -> new ContactCellName(generalClient, imageCacheService)); @@ -394,11 +392,12 @@ private void setupContactTreeTableView() contactTreeTableView.getSelectionModel().setSelectionMode(SelectionMode.SINGLE); contactTreeTableView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { - if (!initiatedByListChange) // We ignore events initiated by sortedList + if (contactListLocked) { - selectedItem = newValue; - displayContact(newValue); + return; } + log.debug("Selection property changed, old: {}, new: {}", oldValue, newValue); + displayContact(newValue); }); treeRoot.setExpanded(true); @@ -411,18 +410,22 @@ private void setupContactTreeTableView() // to save the selection before the sortedList (and then filteredList) are // updated. sortedList.addListener((InvalidationListener) c -> { - initiatedByListChange = true; + log.debug("Sorting invalidated, selected index: {}", contactTreeTableView.getSelectionModel().getSelectedIndex()); + contactListLocked = true; selectedItem = contactTreeTableView.getSelectionModel().getSelectedItem(); }); // Then we restore the selection after filteredList has been // updated. filteredList.addListener((ListChangeListener>) c -> { - initiatedByListChange = false; // Events initiated by sortedList ends up here - if (contactTreeTableView.getSelectionModel().getSelectedItem() == null && selectedItem != null) - { - contactTreeTableView.getSelectionModel().select(selectedItem); - } + // XXX: actually, the selection might be relevant! Check if 'c' changes what is currently being selected? + + log.debug("FilteredList changed, actions: {}", c); + + // XXX: if we don't call this, the selection is lost + contactTreeTableView.getSelectionModel().select(selectedItem); + + contactListLocked = false; }); contactTreeTableView.setRoot(treeRoot); @@ -482,17 +485,6 @@ private void setupMenuFilters() showAllContacts.selectedProperty().set(preferenceService.getPreferences().node(CONTACTS).getBoolean(SHOW_ALL_CONTACTS, false)); } - private void selectOwnContact() - { - contactObservableList.stream() - .filter(contact -> contact.getValue().profileId() == OWN_PROFILE_ID) - .findFirst() - .ifPresent(contact -> { - contactTreeTableView.getSelectionModel().select(contact); - scrollToSelectedContact(); - }); - } - private void getContacts() { Map> contacts = new HashMap<>(); @@ -559,6 +551,7 @@ private void updateProfileWithIdentity(TreeItem profile, TreeItem profile, TreeItem i if (profile.getValue().name().equalsIgnoreCase(identity.getValue().name())) { profile.setValue(identity.getValue()); + refreshContactIfNeeded(profile); return true; } return false; @@ -606,6 +600,7 @@ private void replaceOrAddChildren(TreeItem parent, TreeItem id if (child.getValue().identityId() == identity.getValue().identityId()) { child.setValue(identity.getValue()); + refreshContactIfNeeded(child); return; } } @@ -661,7 +656,6 @@ private void clearCachedImages(TreeItem contact) private void addContact(Contact contact) { log.debug("Adding contact {}", contact); - addedContact = contact; if (contact.profileId() != NO_PROFILE_ID && contact.identityId() != NO_IDENTITY_ID) { @@ -715,6 +709,7 @@ else if (contact.profileId() != NO_PROFILE_ID) { existing.setValue(contact); } + refreshContactIfNeeded(existing); } else { @@ -732,6 +727,7 @@ else if (contact.identityId() != NO_IDENTITY_ID) { clearCachedImages(existing); existing.setValue(contact); + refreshContactIfNeeded(existing); } else { @@ -748,6 +744,7 @@ private void removeContact(Contact contact) { log.debug("Removing contact {}", contact); contactObservableList.removeIf(existingContact -> existingContact.getValue().identityId() == contact.identityId()); + // XXX: unselect if it was selected? } private void updateContactConnection(long profileId, long locationId, Availability availability) @@ -775,13 +772,17 @@ private void updateContactConnection(long profileId, long locationId, Availabili if (existing.isLeaf()) { existing.setValue(Contact.withAvailability(existing.getValue(), availability)); + refreshContactIfNeeded(existing); } else { // There are children, we need to use a different algorithm then. profileClient.findById(profileId) - .doOnSuccess(profile -> existing.setValue(Contact.withAvailability(existing.getValue(), profile.getLocations().stream() - .anyMatch(Location::isConnected) ? Availability.AVAILABLE : Availability.OFFLINE))) + .doOnSuccess(profile -> { + existing.setValue(Contact.withAvailability(existing.getValue(), profile.getLocations().stream() + .anyMatch(Location::isConnected) ? Availability.AVAILABLE : Availability.OFFLINE)); + refreshContactIfNeeded(existing); + }) .subscribe(); } } @@ -847,8 +848,44 @@ private void clearTrust() trust.getItems().clear(); } + /** + * Displays the contact. To be called by the list selector or the own contact display. + * + * @param contact the contact to display + */ private void displayContact(TreeItem contact) { + displayContact(contact, false); + } + + /** + * Refreshes the contact if needed. To be called after each modification of any contact because the listview + * won't do it by itself. + * + * @param contact the contact + */ + private void refreshContactIfNeeded(TreeItem contact) + { + if (displayedContact == contact) + { + displayContact(contact, true); + } + } + + /** + * Displays the contact. Do not call this method directly! Use {@link #displayContact(TreeItem)} or + * {@link #refreshContactIfNeeded(TreeItem)} instead. + * + * @param contact the contact + * @param force to force the refresh + */ + private void displayContact(TreeItem contact, boolean force) + { + if (contactListLocked && !force) + { + return; + } + if (contact == null) { displayedContact = null; @@ -857,12 +894,11 @@ private void displayContact(TreeItem contact) } // Prevent re-displaying the same contact when a different contact was added - if (displayedContact != null && displayedContact == contact && addedContact != null && !addedContact.equals(contact.getValue())) - { - addedContact = null; // Only once - return; - } - addedContact = null; +// if (displayedContact != null && contact.getValue().equals(displayedContact.getValue())) +// { +// return; +// } + displayedContact = contact; hideBadges(); @@ -880,7 +916,7 @@ private void displayContact(TreeItem contact) { typeLabel.setText(bundle.getString("contact-view.information.linked-to-profile")); - fetchProfile(contact.getValue().profileId(), Information.MERGED, contact.isLeaf()); + fetchProfile(contact.getValue().profileId(), Information.MERGED, isSubContact(contact)); fetchIdentity(contact.getValue().identityId(), Information.MERGED); } else if (contact.getValue().profileId() != NO_PROFILE_ID) @@ -929,7 +965,7 @@ private void fetchProfile(long profileId, Information information, boolean isLea showProfileInformation(profile, information); showBadges(profile); setTrust(profile); - trust.setDisable(isLeaf); + trust.setDisable(isLeaf || !profile.isAccepted()); profilePane.setVisible(true); showTableLocations(profile.getLocations()); }))