On Mon, 17 Apr 2023 06:00:19 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> This PR adds the following methods to the `EventTarget` interface:
>> 1. `addEventHandler`
>> 2. `removeEventHandler`
>> 3. `addEventFilter`
>> 4. `removeEventFilter`
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> doc changes
The spec looks fine except for a few missing `@since` tags that need to be
restored, a suggestion on adding `@implSpec` (from Joe), and a couple unrelated
doc changes that should be reverted. See inline.
As for the implementation, the event handling methods in the `Dialog` and `Tab`
classes are new (although they just delegate to an existing instance of
`EventHandlerManager`). Would it be possible to provide a test?
modules/javafx.base/src/main/java/javafx/event/EventTarget.java line 68:
> 66: * @param eventHandler the event handler
> 67: * @throws NullPointerException if {@code eventType} or {@code
> eventHandler} is {@code null}
> 68: * @throws UnsupportedOperationException if this target does not
> support event handlers
Joe Darcy made a recommendation while reviewing the CSR to add an `@implSpec`
tag to the 4 added default methods documenting their behavior, which I think is
a good suggestion. Here is a possible wording:
@implSpec The default implementation of this method throws {@code
UnsupportedOperationException}.
modules/javafx.controls/src/main/java/javafx/scene/control/TableColumn.java
line 85:
> 83: * <li>{@link #editCancelEvent()}
> 84: * </ul>
> 85: *
These doc additions are unrelated to the this PR.
modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableColumn.java
line 80:
> 78: * <li>{@link #editCancelEvent()}
> 79: * </ul>
> 80: *
These doc additions are unrelated to the this PR.
modules/javafx.graphics/src/main/java/javafx/concurrent/Service.java line 744:
> 742: }
> 743:
> 744: @Override
By removing the docs, you lost the `@since` for this method, so you will need
to restore it. You might consider also adding `{@inheritDoc}` to avoid copying
all of the description
modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java line 1252:
> 1250:
> 1251: @Override
> 1252: public final <T extends Event> void addEventHandler(
Same comment in this file about the need to restore the `@since tag`.
modules/javafx.graphics/src/main/java/javafx/scene/transform/Transform.java
line 1915:
> 1913: * <p>
> 1914: * Currently the only event delivered to a {@code Transform} is the
> {@code TransformChangedEvent}
> 1915: * with its single type {@code TRANSFORM_CHANGED}.
You need to restore the `@since`
-------------
Changes requested by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1090#pullrequestreview-1472963662
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224696281
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224894283
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224894379
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224877090
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224877506
PR Review Comment: https://git.openjdk.org/jfx/pull/1090#discussion_r1224881183
