RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]

[Kevin Rushforth]

On Tue, 6 Apr 2021 14:16:03 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> I guess I didn't look closely enough. Mystery solved, though.
>
>> I haven't gone back and done a detailed review yet, but I like the overall changes. The one thing I did notice is that the language used to describe the return value of `unregisterInvalidationListeners` and `unregisterListChangeListeners` is different:
>> 
> 
> just a side-note: technically, both are different because neither the new listChangeListener nor the old changeListener methods are updated yet (we are still at the first bullet of the plan, lazy me will update all to the same as the invalidationListener wording, once we agree on it :)
> 
> So what we are arguing right now is the difference between new (= invalidationListener) and old (= listChangeListener == c&p from changeListener) method doc.
> 
>> ```
>> unregisterInvalidationListeners:
>>      * @return a composed consumer that performs all removed operations, or
>>      *  {@code null} if none have been registered or the observable is {@code null}
>> ```
>> 
>> ```
>> unregisterListChangeListeners:
>>      * @return A single chained {@link Consumer} consisting of all {@link Consumer consumers} registered through
>>      *      {@link #registerListChangeListener(ObservableList, Consumer)}. If no consumers have been registered on this
>>      *      list, null will be returned.
>> ```
>> 
>> I find it confusing to say that the returned list "performs all removed operations". Some might interpret this to mean that is is somehow necessary to call the returned chained consumer as part of the removal, which isn't what you meant to say.
>> 
>> How about something like this for both newly added unregister methods?
>> 
>> ```
>>      * @return a composed consumer consisting of all previously registered consumers, or
>>      * {@code null} if none have been registered or the observable is {@code null}
>> ```
> 
> agree: the _performs_ might be misleading. And I like your suggestion, though I modified it a bit to be consistent with the rest of the changed doc which focuses on "operations" (a term taken from Consumer api): 
> 
>      * @return a composed consumer representing all previously registered operations, or
>      *  {@code null} if none have been registered or the observable is {@code null}

I like this version, using "registered operations".

-------------

PR: https://git.openjdk.java.net/jfx/pull/409