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