On Mon, 17 Feb 2025 22:49:48 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
> > Well, I don't think it is reasonable or desired to have correct old values
> > for these, as it would basically mean we'd need to clone the collection
> > involved to give you a correct old value. The purpose of the old value here
> > would be so you could do a diff and see what's changed, but these
> > properties have their own callbacks for exactly that purpose. IMHO, it was
> > a mistake to base these on properties; at most they should have provided
> > invalidation + their custom diff-style callback.
>
> We could have `ChangeListener` only be called when the list instance is
> changed via `listProperty.set(list)`, but not when the content is replaced
> with `listProperty.get().setAll(list)`. I don't think that the current
> behavior makes any sense at all.
Hm, yeah, I think I could get behind that, as it would actually make some more
sense. Not sure if that would ever be accepted as a change though. At least one
public API (that I could find in a few minutes) documents explicitly that
change listeners will be called for content changes (and I'm sure there is code
out there relying on it). In `ListPropertyBase`:
/**
* Sends notifications to all attached
* {@link javafx.beans.InvalidationListener InvalidationListeners},
* {@link javafx.beans.value.ChangeListener ChangeListeners}, and
* {@link javafx.collections.ListChangeListener}.
*
* This method is called when the content of the list changes.
*
* @param change the change that needs to be propagated
*/
protected void fireValueChangedEvent(ListChangeListener.Change<? extends E>
change) {
ListExpressionHelper.fireValueChangedEvent(helper, change);
}
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2664251427
