On Fri, 2 Feb 2024 13:30:19 GMT, John Hendrikx <[email protected]> wrote:
>> The SimpleSelector and CompoundSelector classes are public classes in an
>> exported package, javafx.css, but they are not intended to be used by
>> applications. They are implementation details. They cannot be constructed
>> directly and no other JavaFX API accepts or returns a SimpleSelector or
>> CompoundSelector.
>>
>> We should deprecate them for removal so we can move them to a non-exported
>> package, removing them from the public API.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Deprecate for 23 and add new method
Looks good, with minor changes.
This also needs a CSR (which has already been requested).
I'll re-approve once we all agree on the new method name.
modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 95:
> 93:
> 94: /**
> 95: * Gets the set of style class names of this Selector. The returned
> set
Maybe just "Gets the immutable set of style class names of this Selector." ?
modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 102:
> 100: * @since 23
> 101: */
> 102: public abstract Set<String> getClasses();
suggestion:
`getStyleClassesSet()`
getClasses() creates expectation of returning `Class<>`es.
modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 200:
> 198: @Override
> 199: public Set<String> getClasses() {
> 200: return
> styleClassSet.stream().map(StyleClass::getStyleClassName).collect(Collectors.toUnmodifiableSet());
minor: could we reformat this on multiple lines, like the other?
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1340#pullrequestreview-1865631005
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480084573
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480083671
PR Review Comment: https://git.openjdk.org/jfx/pull/1340#discussion_r1480081109
