palexdev / VirtualizedFX

GNU Lesser General Public License v3.0
46 stars 4 forks source link

Fix for #5 - VirtualTable - columns header not showing when there are no items #6

Closed stefanofornari closed 1 year ago

stefanofornari commented 1 year ago

This should fix the issue in the subject, please review it. Basically it was not initializing the table if the table contained no items.

palexdev commented 1 year ago

Oh god, where do I start... Thanks for trying, I always appreciate when someone contributes to my projects...however, the fix is completely wrong

1) I'll say this again, and it's the last time. I don't want Maven in my projects, I simply hate it. In general everything that has an XML based syntax sucks to me, but of course it's my personal opinion, people may like it or not, I don't care. To also answer another question from you, what I said also means that I'm not looking for people that are willing to maintain Maven as a secondary build system, I just don't want it 2) The idea of using testfx for testing is good, I'll admit it, I should have done it from the start and I will use this methodology from now on when discovering bugs, but I won't convert the existing 'test apps' to testfx now, it's simply too much work 3) The fix you implemented in TableManager is wrong for several reasons. To begin with, just thinking about that if statement, we don't want to proceed with the table initialization if either the items OR the columns are empty. The table cannot render cells without columns. Also, you didn't try testing the table with a list of items, in fact in such case the test fails 4) To be honest, I already found the fix for the issue when you posted it, but it turned out to be a little bit more complex to implement, if I remember correctly there are changes to do here and there...but then I started working on other things and I forgot to look into it so I'll have to restart the investigation. I understand you need VirtualizedFX urgently, I'll try to do it this same day

stefanofornari commented 1 year ago

Ciao. Sorry, I meant to send only the fix in the PR .... :) thanks for taking care of it!