On Fri, 14 Jan 2022 11:52:40 GMT, Jeanette Winzenburg <faste...@openjdk.org> 
wrote:

> hmm .. I think here are two issues:
> 
> * the auto-sizing code definitely needs to fully configure the cell (with 
> index, row, column, tableView plus applying css) to measure the correct size
> * the implementation of cell.updateItem (even though it's my own ;) is a bit 
> fishy: does its spec really guarantee that getTableRow() != null if !empty()? 
> Meanwhile, I think that's not the case - but then, it's for demonstrating the 
> first bullet :)
> 
> The first is fixed by this PR (looks good on first sight). The second might 
> need a clarification in the method doc .. or not, undecided.

Yes the auto sizing has some more issues. 
While looking on it, I found two problems which should be done as follow-up:
1. Both `resizeColumnToFitContent` methods do not use the row factory of the 
corresponding table (minor, but still something which should be improved here)
2. Currently both implementation add the `TableCell` directly into the 
`TableSkin`, which is not how it is usually done. Normally the `TableRow` is 
added (by the `VirtualFlow`), therefore the example of the related ticket 
[JDK-8251480](https://bugs.openjdk.java.net/browse/JDK-8251480) won't work 
after this fix, but may do when the `TableRow` is added instead (didn't tested 
it though)

May both can be addressed in future via 
[JDK-8251480](https://bugs.openjdk.java.net/browse/JDK-8251480)

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

PR: https://git.openjdk.java.net/jfx/pull/716

Reply via email to