libs-gui icon indicating copy to clipboard operation
libs-gui copied to clipboard

Published 20 hours ago • verified publisher icon gnustep

NSTableRowView class implementation

Open gcasa opened this issue 1 year ago • 1 comments

Recent versions of macOS since 10.7+ use an NSTableRowView to represent a row in NSTableView and it's subclasses. This PR seeks to implement that functionality.

gcasa avatar Apr 25 '24 13:04 gcasa

Work on this might result in refactoring some of the GSThemeDrawing stuff into NSTableView. The methods rowViewAtRow:... and viewAtColumn:row:... might be centric to this process as this seems the best place to manufacture whatever view should be displayed. Just noting this here as I am thinking about the implications of this change.

gcasa avatar Apr 26 '24 15:04 gcasa

@fredkiefer Please take a look at this and let me know. The changes clean up the code a great deal.

gcasa avatar May 03 '24 14:05 gcasa

@fredkiefer let me know what you think please. :)

gcasa avatar May 07 '24 10:05 gcasa

@fredkiefer Hey Fred, can you take a look at this?

gcasa avatar May 16 '24 17:05 gcasa

@fredkiefer Welcome back from vacation, can you take a peek? :)

gcasa avatar Jun 03 '24 17:06 gcasa

@rfm Can you please please review this? I believe that @fredkiefer might still be on vacation. Thanks.

gcasa avatar Jun 05 '24 21:06 gcasa

I am not really sure whether old table views and outline views will still behave as expected with the new code. I hope you are giving that case a lot of testing.

I have. The only time the new code comes into play is when view-based tables are used. I have nstableview tests that are set up to test the normal case in my set of tests. The one that tests non view based tables is here https://github.com/gcasa/NSTableView_test.git. It works as expected. I have also used Gorm to test this since tables are used extensively there.

gcasa avatar Jun 16 '24 10:06 gcasa

Additionally I would not have marked it ready for review if the normal case had any regression at all or if either case in table views or outline views didn't work as expected.

gcasa avatar Jun 16 '24 10:06 gcasa

I believe I have addressed all of the necessary changes.

gcasa avatar Jun 16 '24 12:06 gcasa

@fredkiefer let me know of any additional changes you have. I am going to post pictures of both tests and gorm running to illustrate both work properly.

gcasa avatar Jun 16 '24 18:06 gcasa

Just addressing the comments I already made would be fine.

fredkiefer avatar Jun 16 '24 19:06 fredkiefer

all_tests

gcasa avatar Jun 16 '24 19:06 gcasa

Just addressing the comments I already made would be fine.

Sorry, I thought that I had. :)

gcasa avatar Jun 16 '24 20:06 gcasa

Thank you so much for working with me today!! Your comments are ALWAYS insightful and helpful. @fredkiefer

gcasa avatar Jun 16 '24 21:06 gcasa

Apart from the minor improvements that are still open, this looks good.

I am in the process of making the finale changes you mentioned. Thank you very much for taking the time to go through this with me today.

gcasa avatar Jun 16 '24 23:06 gcasa