libs-gui
libs-gui copied to clipboard
gnustep
NSCollectionView branch -- Adding layouts
Please find the attached xib and enumerated nib. The test for this is at http://github.com/gcasa/NSCollectionViewLayout_test
MainMenu.xib.txt MainMenu-enumerated.nib.txt
In this nib, it appears as though Apple has created a class named NSCollectionViewCore to handle some NSCollectionView responsibilities. Also, it seems that they are using UICollectionView as well. You will see what I am talking about near index 68-71. This nib is "annotated" I used my ruby script to do this, so it is not loadable, just easier to read. :)
This comes from my test for this in the repo here
One of the good things about keyed nibs is that we don't necessarily need to follow everything they are doing.
@fredkiefer
Hi, I am interested in this implementation about layout, what's the plan for the implementation of NSCollectionViewFlowLayout. Can I suppose it is the same design as Apple AppKit?
Yes that’s correct. The idea is to be 100% compatible.
On Mon, Jun 13, 2022 at 03:19 renjwjx @.***> wrote:
Hi, I am interested in this implementation about layout, what's the plan for the implementation of NSCollectionViewFlowLayout. Can I suppose it is the same design as Apple AppKit?
— Reply to this email directly, view it on GitHub https://github.com/gnustep/libs-gui/pull/106#issuecomment-1153563026, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG2J4H3AS2QGMNZSOH5IDVO3OIFANCNFSM46WEFIIA . You are receiving this because you authored the thread.Message ID: @.***>
-- Gregory Casamento GNUstep Lead Developer / OLC, Principal Consultant http://www.gnustep.org - http://heronsperch.blogspot.com https://www.patreon.com/bePatron?u=352392 - Become a Patron https://gf.me/u/x8m3sx - My GNUstep GoFundMe https://teespring.com/stores/gnustep - Store
@fredkiefer Please review and comment when you can. :). I am going to extend my Test at this repo to cover the legacy case and also to start working on the compositional layout code. I have been working on the code in this PR for almost a year and would like to get the existing work merged. I will continue to work on NSCollectionViewCompositionalLayout in the interim on a sub-branch.
It took almost 1.5 years to do (on and off) so an overnight review is not expected (or realistic). I just wanted to let you know it's in a state where it can be reviewed. Take your time it's important to get it right.
In retrospect it could likely have been done in stages but the change was so big to begin with (they basically rewrote the whole class and left the legacy code in place) that it was difficult to say where the lines should have been drawn.
As it is I am leaving the NSCollectionViewCompositionalLayout class as a stub in this PR since it would only add tons more and is a very complex class. GC
@fredkiefer as always, you have a keen eye. Thanks for your comments thus far. I will get to them asap.
@fredkiefer I am going to attempt to address the remainder of your concerns on this PR. Please let's try to do a review sometime before the meeting this weekend. Thanks, GC
@fredkiefer I believe I have adequately answered all concerns.
I see plenty of comment that where not addressed. If you want to ignore them go ahead and merge. Just don't state that they were answered. You might have to display the hidden items. Github is trying to compress the history.
@fredkiefer I believe I have adequately answered all concerns.
I see plenty of comment that where not addressed. If you want to ignore them go ahead and merge. Just don't state that they were answered. You might have to display the hidden items. Github is trying to compress the history.
Instead of, yet again, being falsely accused of merging prematurely, I will review the changes again even though this PR has been pending for over a year and will attempt to make sure that your concerns have all been addressed.
In this nib, it appears as though Apple has created a class named NSCollectionViewCore to handle some NSCollectionView responsibilities. Also, it seems that they are using UICollectionView as well. You will see what I am talking about near index 68-71. This nib is "annotated" I used my ruby script to do this, so it is not loadable, just easier to read. :)
This comes from my test for this in the repo here
One of the good things about keyed nibs is that we don't necessarily need to follow everything they are doing.
@fredkiefer
Just wanted to make sure you see this. Again the test is here https://github.com/gcasa/NSCollectionViewLayout_test
@fredkiefer All I see left is our disagreement about _selectionIndexPaths and _selectionIndexes. I will do further research to see if there is, indeed, a relationship between these two variables. Please refer to the test I have referenced a number of times.
I just added this test... https://github.com/gcasa/NSCollectionViewArray_test it seems we have some deeper concerns. This test works on macOS but does NOT work correctly on GNUstep. This means that Testplant (now Keysight) wrote this class so that it worked for Eggplant/EPF and didn't test it in the general case. SMDH... I am continuing to do research to see if those two variables are related, but this is an interesting detail to add. I am not sure if I will take care of all of the issues with their implementation of the legacy NSCollectionView support in this PR. We will save that for a later discussion.
I am wondering if I couldn't do something like this...
NSMutableIndexSet *indexSet = [NSMutableIndexSet indexSet];
for (NSIndexPath *indexPath in indexPathsSet) {
[indexSet addIndex:indexPath.row];
}
NSIndexSet *convertedIndexSet = [indexSet copy];
I have noticed, in the exception, I sent you, that there is a suspiciously named function...
2023-02-11 11:23:46.959590-0500 NSCollectionViewLayout_test[35647:19643369] *** Assertion failure in NSIndexSet *_NSIndexSetFromCollectionViewIndexPaths(NSSet<NSIndexPath *> *)_block_invoke(), NSCollectionView.m:9000
It seems there might be a way to keep them in sync. It's not obvious from the documentation that this is the case, but I don't think it hurts to implement it. I have been unable to prove a relationship either way.
As additional proof
2023-02-12 11:31:58.746793-0500 NSCollectionViewLayout_test[71987:20745783] selectionIndexes: <NSMutableIndexSet: 0x600003527840>[number of indexes: 1 (in 1 ranges), indexes: (1)]
2023-02-12 11:31:58.746848-0500 NSCollectionViewLayout_test[71987:20745783] selectionIndexPaths: _NSCollectionViewMutableIndexPathSet {0:(1)}
when the datasource method numberOfSectionsInCollectionView: returns 1, the exception does not occur and, as you can see, they are synchronized. The GS code does the same thing now with the methods I added yesterday.
I have modified the layout test here to illustrate this. I have left a comment in it explaining.