[Review request] Adding pre-built cells and cell factories to JavaFX

[Jonathan Giles]

setCellFactory is, and will remain, on the ListView, TreeView and 
TableView controls. Richards question was not targeted at this 
particular API.

Instead, he was questioning the need for, say, a TextFieldCellFactory 
class, that has a bunch of forListView(), etc methods (forListView(), 
forTreeView(), and forTableView(), with a bunch of different acceptable 
parameters). Richard's suggestion was to move all the TextField listView 
cell factories to the TextFieldListCell class, all the TextField 
TreeView cell factories to TextFieldTreeCell, etc.

This is closer to where the cell factories belong, but it comes at the 
cost of distributing the cell factories to a number of different 
classes, rather than centralising them. The main gain was the removal of 
four classes, where every class has a cost that we are very conscious of.

-- Jonathan

On 9/05/2012 5:35 p.m., Claus Luethje wrote:
> I prefer having the setCellFactory() methods attached to controls, because it shows the connection between the control and the defined cell factories better. In my view, it enhances readability of the code.
> Just my 0.02$
>
> Claus
>
> -----Original Message-----
> From: openjfx-dev-bounces at openjdk.java.net [mailto:openjfx-dev-bounces at openjdk.java.net] On Behalf Of Jonathan Giles
> Sent: Mittwoch, 9. Mai 2012 02:11
> To: Richard Bair
> Cc: openjfx-dev at openjdk.java.net
> Subject: Re: [Review request] Adding pre-built cells and cell factories to JavaFX
>
> The reason for the current approach is, as always, discoverability of API. But I never seem to win with that argument either :-)
>
> I am totally happy with having the API on the specific cell classes themselves, or in their current location. I don't have a strong gut feeling either way, so hopefully others may chime in with their preference. If not, I'll make the switch tomorrow.
>
> -- Jonathan
>
> On Wednesday, 9 May 2012 11:01:02 a.m., Richard Bair wrote:
>> Jonathan, I was just thinking (I know we talked about this before some time back but I don't remember the reasoning), why do we have the ***Factory classes? Why not just move those factory methods onto the appropriate Cells? So it would be:
>>
>> listView.setCellFactory(TextFieldCell.cellFactory(...));
>>
>> Just thinking four fewer classes, and associating the factory methods with the class itself. What do you think?
>>
>> Richard
>>
>> On May 8, 2012, at 3:29 PM, Richard Bair wrote:
>>
>>> I've been over this a few times with Jonathan and I think the API is really quite nice. I would especially like to hear any feedback from folks using FXML, from SceneBuilder, and from anybody who gives it a try. The API is essentially what Jonathan had done in DataFX and has had a significant amount of usage already so we're pretty confident that it is friendly to use :-).
>>>
>>> Thanks
>>> Richard
>>>
>>> On May 8, 2012, at 3:14 PM, Jonathan Giles wrote:
>>>
>>>> Hi all,
>>>>
>>>> http://javafx-jira.kenai.com/browse/RT-19452
>>>>
>>>> This jira feature request asks for JavaFX to include a number of pre-built cell factories for use in ListView, TreeView and TableView. This was always intended functionality, it just took some time to get around to adding it. These cell factories are intended to support functionality such as editing by TextField, ChoiceBox, ComboBox, and CheckBox, easily converting objects into strings, and values into progress bars.
>>>>
>>>> I intend to move com.sun.javafx.scene.control.CheckBoxTreeItem into javafx.scene.control.
>>>>
>>>> The classes below I intend to move to javafx.scene.control.cell:
>>>> com.sun.javafx.scene.control.cell.CheckBoxCellFactory
>>>> com.sun.javafx.scene.control.cell.CheckBoxListCell
>>>> com.sun.javafx.scene.control.cell.CheckBoxTableCell
>>>> com.sun.javafx.scene.control.cell.CheckBoxTreeCell
>>>> com.sun.javafx.scene.control.cell.ChoiceBoxCellFactory
>>>> com.sun.javafx.scene.control.cell.ChoiceBoxListCell
>>>> com.sun.javafx.scene.control.cell.ChoiceBoxTableCell
>>>> com.sun.javafx.scene.control.cell.ChoiceBoxTreeCell
>>>> com.sun.javafx.scene.control.cell.ComboBoxCellFactory
>>>> com.sun.javafx.scene.control.cell.ComboBoxListCell
>>>> com.sun.javafx.scene.control.cell.ComboBoxTableCell
>>>> com.sun.javafx.scene.control.cell.ComboBoxTreeCell
>>>> com.sun.javafx.scene.control.cell.ProgressBarCellFactory
>>>> com.sun.javafx.scene.control.cell.ProgressBarTableCell
>>>> com.sun.javafx.scene.control.cell.TextFieldCellFactory
>>>> com.sun.javafx.scene.control.cell.TextFieldListCell
>>>> com.sun.javafx.scene.control.cell.TextFieldTableCell
>>>> com.sun.javafx.scene.control.cell.TextFieldTreeCell
>>>>
>>>> In addition to these classes, I am also introducing other, related classes, including:
>>>> javafx.util.converter.FormatStringConverter
>>>> com.sun.javafx.scene.control.cell.MapValueFactory (which will move
>>>> to javafx.scene.control.cell to work alongside the
>>>> PropertyValueFactory)
>>>>
>>>> To support the CheckBox inside a TreeView case, I'm including the CheckBoxTreeItem class that can ensure that parent and children tree items are updated when a tree item is selected or deselected.
>>>>
>>>> I have included considerable javadoc documentation already, but I intend to add more soon to include a number of worked examples on how to use this new API.
>>>>
>>>> I look forward to your feedback.
>>>>
>>>> -- Jonathan