RFR: JDK-8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations
Marius Hanl
mhanl at openjdk.org
Thu Feb 15 12:16:03 UTC 2024
On Wed, 14 Feb 2024 20:57:14 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> Yes! I did it this way to be consistent with all other cell implementations. I really think that this should be refactored, I can also create a ticket for this. My preference would be to be consistent now, but change it for all cell implementations in one go.
>> What is your opinion?
>
> my opinion: the code is not broken per se. Since this block is edited, I would have changed it just in this file to make the logic clearer (in other places, like TreeTableCell:685 it is functional because we have more code in line 692 and on).
> I don't think another ticket is necessary because, technically speaking, the code is not broken.
> (just my opinion)
yes, but other cells have the exact same pattern, although it is not needed. And technically speaking, this is not even needed in `TreeTableCell`, tbh I never even needed a labeled if ever. 😄
But sure, can also change.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1360#discussion_r1490919877
More information about the openjfx-dev
mailing list