RFR: 8345348: CSS media feature queries [v13]

Michael Strauß mstrauss at openjdk.org
Mon Apr 14 17:57:50 UTC 2025


On Mon, 14 Apr 2025 11:34:43 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   improved implementation of NullCoalescingPropertyBase
>
> modules/javafx.graphics/src/main/java/javafx/css/Rule.java line 64:
> 
>> 62:     private final MediaRule mediaRule;
>> 63: 
>> 64:     private List<Selector> selectors = null;
> 
> I dislike the accessor pattern somewhat; I wonder, would it be possible to:
> 
> - Make `Rule` sealed: `public abstract sealed class Rule permits InternalRule` (*) similar to how this was done for `Selector`: `public abstract sealed class Selector permits SimpleSelector, CompoundSelector`
> - `InternalRule` would not be public API, and can therefore introduce a public method to get `MediaRule`
> - `Rule` does not have a public constructor, so IMHO there's no risk in there ever being an instance of `Rule` directly (similar reasoning was used to when the `SimpleSelector` / `CompountSelector` refactoring was done).
> 
> This way getting access to the `getMediaRule` method is just a simple `instanceof` check away:
> 
>      MediaRule rule = rule instanceof InternalRule ir ? ir.getMediaRule() : null;
> 
> (*) `InternalRule` is just a place holder name

I can see where you're coming from, and this really makes sense when you're having a public interface and an internal implementation. But that's not the case here, the implementation is out there in `javafx.css`. I don't mind the accessor pattern here (it's simple enough, and we can change it at any time), and would rather revisit the entire public CSS API at some point and either remove it or make it useful.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1655#discussion_r2042636942


More information about the openjfx-dev mailing list