On Mon, 13 Oct 2025 12:19:52 GMT, Marius Hanl <[email protected]> wrote:
> This is an initial poc for the long standing issue of allowing to commit a > cell value when the focus is lost or the editing index (cell) changed. This > also contains [JDK-8089311](https://bugs.openjdk.org/browse/JDK-8089311) (for > better understanding the usecase with `TextField` cells, but we may split > this later on). > > API > - > - Instead of calling `cancelEdit`, every cell now calls `stopEdit` when the > focus is lost or the editing index changed. The default behavior is > cancelling the edit, but developers can now override the behavior and allow a > `commitEdit` instead > - There are multiple 'events' that can lead to a editing change. Every change > will now call `stopEdit`. > It is therefore the responsibility of the developer to decide, when it makes > sense to actually commit the value instead of cancelling it. This decision > was made as the behavior is manipulating the editing index, but you as a > developer can as well. We do not really know what intention led to e.g. a > change of the editing index. > - Every `MOUSE_PRESSED` shifts the focus to the cell container, which is > undesired in case of editing the cell. So this event is now consumed. The > cell container will request focus before (if it is not yet set) > - All `TextField` cells now commit their value (instead of cancel) on focus > loss > - `TextField` Escape handling was badly implemented (it was never really > called, as the cell container handled Escape before) > > Considerations > - > - I tried to make the API minimal, and without breaking changes (other than > the `TextField` cells committing their values, but we may split this up) > - The Cell Container focus behavior is, well, weird right now. That is why > consuming the event is needed to better support this PR. One thing we may can > consider is using the `focusWithin` property instead for all 4 Cell > Containers and not calling `requestFocus` for nearly every `MOUSE_PRESSED` > event. If we decide so, this is needs to be done before merging this PR. > - Clicking the `ScrollBar` now commits/cancels the edit. I checked other > applications and this is very common. But something I need to note here. This > probably can be fixed in the same way mentioned above (`focusWithin`) > - It might be hard for a developer to exactly know the cause why `stopEdit` > is called. This does not seem like a problem, as e.g. for a `TextField`, you > normally register listeners for e.g. pressing the Escape key on it, so you > keep full control. > > Another Approach > - > - Another Approach I tested could be to request the focus... @andy-goryachev-oracle I wonder if a new page in the monkey tester with a table and various controls to test out focus loss makes sense? Just an idea though :) Copying my text from the mailing list: I did a lot of focus tests the last weeks and wrote a big sampler application as well. I will provide the source code as Gist in the PR description at one point. First of all, things look pretty good! There are some problematic cases, and at least one bug from what I can see. Note: All the problematic cases below also affect all other Controls that commit their value on focus loss, like DatePicker, Spinner, ... What works: - By default, the focus loss commit works well with all Controls out of the box - Mnemonics work. They will first request focus before they trigger the action Problematic cases: - A Tab selection change is fired before the focus is requested on the TabPane. While the focus lost commit still works, it is in my opinion too late. Think about disabling a Tab when something in a Table is invalid (which we know after the commit) - List/Tree/Table/View: Selection and focus is changed before the actual cell container will receive the focus. Even more weird: For both Tables, the selection change is fired BEFORE the focus change. List- and TreeView have the correct order. Again, in case we want to disable this Control when another Table is invalid, which we know right at the commit, we will still first trigger a selection and focus change Buggy cases: - As John also mentioned, non focus traversable Controls are completely broken. A non focus traversable Button or CheckBox will not request any focus. Therefore, Cells, DatePicker, ... can not commit their value. Other Controls set to non focus traversable like TextField or the TextField inside the DatePicker will request focus, the DatePicker button will not. This seems like a bug to me. Could also be something we want to ignore, as we can say: In this case, all focus loss commits are broken, so it is up to you. Feedback welcome Other cases: - MenuBar, Menu, MenuItem will not trigger focus at all. This might be expected. I don't know what to think about that. Feedback welcome The application for testing commit on focus lost: <img width="1243" height="650" alt="image" src="https://github.com/user-attachments/assets/61770d10-18f0-423a-b74a-c98610cd0c08"; /> https://gist.github.com/Maran23/7f1cef17b3ed638f3b9c65f911a8b390 I just did some retesting, and everything works as expected. I think this is ready for review. All issues that I found do exist with other commit-on-focus-loss `Control`s as well (`ComboBox`, `DatePicker`, `Spinner`). So there really is no blocker for this feature. Note: I will eventually search/create tickets for my focus findings (and if I have even more time, fix them). So those are on my list. But again, no blocker for this feature! :) ------------- PR Comment: https://git.openjdk.org/jfx/pull/1935#issuecomment-3406894588 PR Comment: https://git.openjdk.org/jfx/pull/1935#issuecomment-4048456415 PR Comment: https://git.openjdk.org/jfx/pull/1935#issuecomment-4048730997
