On Thu, 17 Jul 2025 05:18:28 GMT, Michael Strauß <[email protected]> wrote:

>> Implementation of [focus 
>> delegation](https://gist.github.com/mstr2/44d94f0bd5b5c030e26a47103063aa29).
>
> Michael Strauß has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Avoid variable reassignment

So I wasted nearly my whole weekend trying to see how we could get rid of 
`setFocused(..)` (deprecate) and fix everything that needs it. I came across 
nearly all corner cases we have now, and checked all Controls and their 
traversable behavior.

Those are all my notes I wrote while doing so, straight up copying them here. 
Maybe they are useful.

Requirements:
- MANUALLY synchronize Focus from ComboBox/DatePicker/Spinner also into 
FakeFocusTextField. But rather than being a focus owner, it should just 
synchronize the focus so that it can react correctly (caret etc.) and also look 
correct (focused, CSS).
- RenderThemeImpl: Should synchronize focus from DOM state into the component 
without requesting real focus. Again, this is the same usecase as above. We 
want to manually synchronize focus so that the Controls will react correctly 
(caret etc.) and also look correct (focused, CSS)
- Cell: This one would be completely avoidable, but we cannot break it now. 
This is only done to cancel (or soon, stop) edit and for CSS. Both could easily 
be implemented on their own (Like TabPane)
- Some Controls need to know if something else was interacted with (focus 
changed) so they commit or cancel their value
- Complex Control like ScrollPane or Table want to always show the focused 
rectangle even though sub controls in there are focused, like the ScollBar or 
Row/Cell/Cell TextField. This can be achieved by making those not focus 
traversable and using focusWithin. As focusWithin was not available when the 
API was made, those Controls are usually requesting their focus back into the 
Complex Control when received or rely on hacks. And they do it wrong, e.g. 
VirtualFlow sometimes receives focus when scrolling
- There are Controls like Accordion or ScrollPane that never want to have 
focus, only their TitledPanes/Content should have it (It still can be enforced 
by setting focus traversable back to true, which is a bit weird)

Disadvantages:
- Due to the nature of how ComboBox/DatePicker/Spinner are built up, they need 
various hacks: FakeFocusTextField, redirecting events and overwriting the 
traversal engine.
- Committing or Cancelling based on the focusedProperty with focusTraversable 
set to false is not possible and not even clear if it should. You may want to 
do something like writing something in a TextArea and click on Buttons in a 
ToolBar to make text bold, which should not shift focus away and not commit
- Making some Controls focus traversable like Accordion will suddenly focus 
Accordion, which is not really visible (no indicator as well), makes no sense 
and even worse, you are getting focus trapped (tested today)
- As Cells inside a Table are focused with hacky `setFocused,` they will 
contribute to focusWithin, which will make the the table focusWithin true even 
though something completely different outside the Table is now focusOwner 
(focused)

So what do we need then:
- Some Controls need a manual focus sync (from A -> B), but will never become 
focus owner and should not contribute to focusWithin and focusVisible. They 
also should not be traversable to and should receive the events correctly.
  - In this case, the Control should be a blackbox, and everything under it 
should be not accessible with focus and focus traversable, while still 
receiving events if needed.
  - This seems like a strong candidate for focus delegation, but it would be 
good to keep it simple. Even keeping FakeFocusTextField with the focus sync and 
redirecting the key events in a smart way (not like now), while everything else 
is fixed (no custom traversal engine) seems like a good compromise
- Some Controls are rather a whitebox, never want focus and never be 
focus-traversable. Everything it contains should be accessible and focus 
traversable. 
  - Basically focus delegation reversed somewhat
  - This is a bit weird, because they have set focus traversable to false, but 
we can set them focus traversable and they behave completey weird and it makes 
no sense, even trapping focus
    - Maybe it makes more sense that those SHOULD be focus traversable, but 
that only means that their children can be. Making e.g. the ScrollPane not 
focus traversable means the content can not be access with TAB, even if the 
content is focus traversable.
- Complex Controls like the Table are in between black and whitebox. Most of 
the children should also never be focused, never be focus traversable, while 
some may should. Example: Column Header for sorting/filtering or cells
  - Focus Rect can be solved easily with focusWithin
  - Cells may should receive focus and be focus owner, finalizing the weird 
implementation we have now and making it complete. FocusWithin will make sure 
the everything still looks fine

Conclusion:
- We need a way to control (for a `Control`) which children are accessible and 
which not, without hacks, traversal engine hacks, key redirection hacks, 
messing up focusWithin and still be focusable (manually), even if there is a 
little bit of manual work required for manual focus and key events.
- A `Control` should specify if it itself can receive focus / be focus 
traversable (so the Control itself can get focus) or just redirect it 
completely (focus traversable is true, but it will not be focused itself but 
the children)
  - Accessible children can be traversed to and receive focus and key events as 
normal
  - Non Accessible childrens can not be traversed to and do not receive focus, 
unless it is manually synced. They will receive key events manually - we need 
to specify that as well
  - Non accessible children can request focus (there is no way for us to forbid 
that probably, but it may break expectations when further pressing TAB or 
SHIFT+TAB. 
    - We may can implement a warning for this case, although I would like to 
not do a search in the Scene Tree' to MAY find a mismatch and make everything 
more complex and slow
    - In general, I would like to avoid doing complex 'search the Scene Tree' 
operations for that
 
The biggest motivation is to have a straight baseline which solves all the 
cases, not just one. I think all those cases do share a somewhat common problem.
I think it should be okay to use other Controls inside the Skin. 
I also think it should be possible for us JavaFX Users to write a Control and 
Skin that can also facilitate whatever we come up with. 
This would make writing own Controls with Skins much easier. Whatever we learn 
should also influence the behaviors.

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

PR Comment: https://git.openjdk.org/jfx/pull/1632#issuecomment-4149030411

Reply via email to