RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses [v9]
Michael Strauß
mstrauss at openjdk.org
Wed Jul 6 23:44:51 UTC 2022
On Wed, 6 Jul 2022 22:17:52 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 23 additional commits since the last revision:
>>
>> - Merge branch 'master' into feature/focusvisible
>> - Updated since tag
>> - Update copyright headers
>> - Merge branch 'master' into feature/focusvisible
>> - fixed undeterministic test failures
>> - minor wording change
>> - restart github actions
>> - Merge branch 'master' of https://github.com/mstr2/jfx into feature/focusvisible
>> - changes per discussion, added test
>> - wording
>> - ... and 13 more: https://git.openjdk.org/jfx/compare/7cf8f2d0...0c7d6e72
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 8191:
>
>> 8189: */
>> 8190: private void updateRemovedParentFocus(Node oldParent) {
>> 8191: if (oldParent != null && focusWithin.get()) {
>
> I see that you aren't calling `this.focusWithin.set(false)`. Is that because in the case of the node that was removed from its parent, it will have its `focused` property set to false (as a result of it being removed from the scene), which will then set its `focusWithin` to false?
Yes, that's correct. A node that is removed from a scene graph cannot be focused, which also means that it can't have focusWithin.
> modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 4056:
>
>> 4054:
>> 4055: private void setFocusVisible(Node node, boolean focusVisible) {
>> 4056: Node.FocusPropertyBase property = (Node.FocusPropertyBase)node.focusVisibleProperty();
>
> Did you consider using a similar pattern to `ReadOnlyBooleanWrapper` to avoid the cast? Given that an application can't call the set method (either by casting or by using reflection), I don't see a problem with this, so it is just a question.
I removed the cast by making the `Node.focusVisible` field package-private. That should make it clearer that the property is actually set from outside the `Node` class.
-------------
PR: https://git.openjdk.org/jfx/pull/475
More information about the openjfx-dev
mailing list