RFR: 8301302: Platform preferences API [v5]
Michael Strauß
mstrauss at openjdk.org
Tue Sep 5 23:26:50 UTC 2023
On Tue, 5 Sep 2023 19:30:30 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Removed application preferences implementation
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PreferenceProperties.java line 118:
>
>> 116: }
>> 117:
>> 118: fireValueChangedEvent();
>
> It looks like this will fire `colorProperty.fireValueChangedEvent();` for all colors in `allColors`, even when none has changed.
> edit: I see what you did here. Contrary to it's name, `fireValueChangeEvent` does not always fire. Perhaps we could rename the method to fireValueChangeEventIfChanged/Maybe or some such?
I've renamed it to `fireValueChangedIfNecessary`.
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PreferenceProperties.java line 197:
>
>> 195:
>> 196: private void updateEffectiveValue() {
>> 197: // Choose the first non-null value in this order: overrideValue, platformValue, defaultValue.
>
> are null default values allowed?
I can't think of a good reason why we would have preferences with a `null` default value. The small set of convenience properties is meant as a baseline that all JavaFX developers can rely on.
> tests/manual/events/PlatformPreferencesTest.java line 113:
>
>> 111: return "{\r\n\t" + entries + "\r\n}\r\n";
>> 112: }
>> 113:
>
> extra newline?
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1316504358
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1316505649
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1316506587
More information about the openjfx-dev
mailing list