[Review request] Make Cell.updateItem(item, empty) public, rather than protected

Jonathan Giles jonathan.giles at oracle.com
Mon Jul 8 19:10:01 PDT 2013

Hi all,

This request is to change the API for Cell.updateItem(T item, boolean 
empty) from protected to public. Clearly this will not be a breaking 
change, but it does make the API more public than ideal :-)

I've not discussed this issue with anyone else, so this is a rather 
early review request seeking peoples first impressions. The driving 
reason for this request is RT-31274 (link below). The general issue that 
RT-31274 highlights is that ComboBox can not properly render custom 
'button cells' (i.e. the cell that shows inside the button, rather than 
the dropdown list) when the updateItem method is overridden. This is 
because the ComboBox skin can't call updateItem, and therefore currently 
falls back to calling setText(text) and setGraphic(null) (which works 
well when the developer has set a StringConverter, as this can be done 
in the skin).

I should note that this will bring Cell.updateItem(..) in line with 
IndexedCell.updateIndex, which has been public for as long as I can recall.

Because of RT-31274, my current thinking is to make the following changes:

1) Make Cell.updateItem(T item, boolean empty) public, rather than 
protected. It will continue to be 'expert' API that should not be called 
(as is the public setItem(T item) method, updateIndex(..), etc).

2) Update ComboBoxListViewSkin to continue calling setText(...) and 
setGraphic(null), but then also subsequently call updateItem(item). This 
will have the effect of not breaking existing users, but also give 
custom button cells a chance to draw themselves properly. Of course, if 
these custom cells depend on the ComboBox string converter they will 
have to handle this themselves (but, of course, the proper converted 
string will already be in the Cell text property, so this shouldn't be a 

An alternative approach is to override the Cell.item invalidated() 
method such that this calls down to updateItem(...), but that seems a 
little magic and a lot smelly, so I'd definitely rather not go down that 



More information about the openjfx-dev mailing list