Commit on focus loss in ListView, TreeView, TableView, and TreeTableView

[Jonathan Giles]

Hi all,

In the next week I have two topics to discuss, both with similar names 
but related to quite different functionality. The topic for today is 
adding the necessary API to the JavaFX UI control Cell class to enable 
support for 'commit on focus loss', that is, the ability to 
automatically commit a user-input value (into, say, a TextField inside a 
Cell) back into the backing data structure when the cell (or its 
content) looses focus. The topic for _next_ week relates to a new focus 
traversal API I plan to propose, related to focus traversal over all 
nodes in the scenegraph.

At present, the cells used in ListView, TreeView, TableView, and 
TreeTableView all support the concept of switching between a normal 
state and an editing state. Due to the way cells were built, when the 
editing state was enabled, the developer was simply entitled to change 
the controls displayed to the user. The cell is never made aware of what 
the editing controls were, and as such has no way to know what the user 
has entered. Today, when focus is lost, this results in a cancel edit 
event being fired, the user input is dismissed, and the cell returns to 
its non-editing state displaying the previous data. This is less than 
ideal, and is what we are trying to fix here.

Unfortunately, there is no way to enable this 'for free'. If we used 
heuristics and looked at the nodes in the cell when in the editing 
state, it would be error-prone. For one, what if the cell has, say, 
multiple TextFields? Even in the simple cases, what if the data model 
was not String-based, and required some conversion from String to T? 
What if the user had a custom cell for editing that could not be 
understood by the Cell? We'd have no choice but to offer some form of 
fallback API to support this.

Therefore, my proposal is to flip this around - we introduce a single 
new method on the Cell class, as shown below, and then retrofit our 
custom cell factories (most notably the TextField*Cell classes) to 
implement this method, providing the commit-on-focus-loss behavior that 
is expected.

     /**
      * Developers of custom cell implementations should override this 
method when
      * the cell provides editing functionality, with this method 
returning the
      * user input after the user has interacted with the editing 
components.
      * The form of the returned data (wrapped in an {@link Optional}) 
should
      * be the same as the form of the underlying data model, hence the
      * use of the {@code T} generic type. If no value is available, or 
if the
      * value to be returned is invalid, {@code Optional.empty()} should 
be returned
      * to indicate that the commit should not proceed..
      *
      * @return The value provided by the user into this cell when it 
was in its
      *      editing state, in the form of the underlying data model. If 
the value
      *      is invalid or unable to be determined, {@code 
Optional.empty()} should
      *      be returned.
      * @since 10
      */
     protected Optional<T> getEditorValue() {
         return Optional.empty();
     }

As noted, this method is already overridden by the pre-built 
TextField*Cell classes (so by default, users of TextFieldListCell, 
TextFieldTreeCell, TextFieldTableCell, and TextFieldTreeTableCell get 
commit on focus loss by default). Developers of custom cells who want 
commit on focus loss to be enabled will have to override this method. I 
will work to ensure that this new method is well-understood and widely 
known, and that all relevant documentation I can change will be changed 
to mention this.

As part of this work, I have also developed a test app - 
HelloCommitOnFocusLoss - that tests this new functionality on ListView, 
TreeView, TableView, and TreeTableView. In all tests that I have 
implemented so far, I can see that commit on focus loss works in cases 
where the user clicks on a new row, and when they give focus to another 
node (by both mouse and tab key). I am happy to extend this test 
application with any test cases people fear may not be supported, and 
then we can find gaps in this implementation (or, hopefully, be 
pleasantly surprised that these cases are already being met).

Discussion can take place in the Jira issue at [1], and the webrev with 
my proposed changes is at [2], although note that I have yet to develop 
enough unit tests for this change - as I wanted to discuss the API first.

Your feedback now is much appreciated.

[1] https://bugs.openjdk.java.net/browse/JDK-8089514
[2] http://cr.openjdk.java.net/~jgiles/8089514.1/

Thanks,
-- Jonathan