Recalculate contentOffset for device rotation

[whatcher2]

On very large tables with different layouts in portrait/landscape like 3x4/4x3 the total grid height can be very different so its necessary to change the contentOffset proportionally. This keeps the current cells in view (as much as possible). Before making any calculations the cell size needs to be fetched to handle cases when its height differs between portrait and landscape.

[evadne]

Thanks for your contribution. I have a strong feeling that code like this is very useful, but shall live within the view controller. Bounds changing do not necessary reflect layout orientation change, as in the grid view might scale only in one direction. This code introduces too much unexpected behavior for my taste. We might benefit from putting this in a sample project instead of in the grid view itself. Another idea is to add a flag on the grid view that handles this special case.

I’ll be closing this issue for now but feel free to re-open it to discuss. Thanks again!

[whatcher2]

Hi Evadne,

I'm not sure my submission comments got the point across. The AQGridView is broken when used with a large table size (lots of data) AND ( the cell height or number of cells per row differ between orientations). So for example consider a AQGridView with 2000 rows and in portrait 1 cell per row and in landscape 2 cells per row (cell height is the same) . In landscape the content height is 1/2 that of portrait. Scroll down 1/2 way when in portrait. When the device is rotated to landscape the class displays the last (or nearly last) cell in the table because the content offset is not adjusted correctly for the change in layout.

"Bounds changing _do not necessary reflec_t\ layout orientation change," is true but in cases when it is true you would leave the fix to the user of the class? Apple handles this inside the UITableView class for users of the class. If the cell height is shorter in landscape, the content height of the table decreases and the content offset is changed as well. This results in the same cells displayed to the user in either orientation. This is expected. AQGridView does not have this expected behavior.

Someone with more experience or knowledge of the AQGridView's code base than I have can point to a better place to fix this than I suggested and improve upon it but " live within the view controller" , "sample project", and "add a flag on the grid view that handles this special case" are not solutions that fix the bug for the user of the class. What is the "unexpected behavior" specifically? I'd be happy to fix bugs with my submission.

It seems to me that if you refuse to fix this behavior inside the class you may as well stop taking contributions to it. The class makes a fundamental improvement to UITableView - support more "cells" per row - BUT IT DOESN"T WORK when the number of cells per row differs with orientation changes. If thats not a fundamental reason to use the class in the first place, it certainly is the next logical use case.

How about reopening this and we discuss a better way to make the fix rather than have me supply an example that works around the problem?

Walter

On Fri, Sep 14, 2012 at 9:40 PM, Evadne Wu notifications@github.com wrote:

Thanks for your contribution. I have a strong feeling that code like this is very useful, but shall live within the view controller. Bounds changing do not necessary reflect layout orientation change, as in the grid view might scale only in one direction. This code introduces too much unexpected behavior for my taste. We might benefit from putting this in a sample project instead of in the grid view itself. Another idea is to add a flag on the grid view that handles this special case.

I’ll be closing this issue for now but feel free to re-open it to discuss. Thanks again!

— Reply to this email directly or view it on GitHubhttps://github.com/AlanQuatermain/AQGridView/pull/156#issuecomment-8580764.

[claybridges]

@evadne @whatcher2 @AlanQuatermain Yeah, I find this reason for closure questionable.

I can understand, e.g., wanting to be pure regarding Controller/View division of labor, if the existing classes already exhibited that purity; by my reading they don't. But, even if by your measure, they do, a helpful reply would suggest where in the controller the fix might go, and why, and leave the request open. The sample code suggestion (for a bug fix) seems just whack.

Please reconsider.

[evadne]

Sorry — my fault in not communicating clearly. Showing some other cells, while at the same time maintaining offset, is certainly bad user experience and will nevertheless require code either inside or outside the project to handle. The original goals of the project is to match NSCollectionView as much as possible. It conflicts with optimizing for developer happiness.

For a very quick testing, iBooks exhibits the same behavior on device rotation — it shows a different set of cells. The Photos app on iPad handles this case better, but I fail to find any app where, after rotating to and back from a certain orientation, the grid view shows the same set of cells. For example, go to Photos.app, open the Photo Stream, go landscape, navigate to the last row and scroll one row downwards. Now rotate to portrait. The app should show the last photo on the last row. Rotate back to landscape and the grid view now shows the last cell as well.

Judging from code and how the sample app runs, we’ll get the behavior that Photos.app show in AQGridView.

As an additional example why a simple fit-to-bottom or fit-to-displayed-index-paths will sometimes fail, enable the Emoji keyboard on the iPhone, then go to any app that supports landscape input. Scroll to the last page in the segment, then start rotating the device to landscape and back. You end up on the first page of the segment.

The desired behavior is not consistently handled even across iOS apps and it’s important for it to be well-defined. However, it’s always okay for AQGridView to have its own idiosyncrasy — as long as it is very reasonably documented — and I surely have made a judgmental mistake of rejecting a patch for a certain defect.

I’d like to pull this into develop shortly, test on demo, internal and public projects, and wait for community feedback.

Thanks again for your feedback.

[whatcher2]

Sounds great, thanks!

For what its worth iBooks as you described it sounds broken to me. Apple has their share of bugs, too. And yes it's not possible to have exactly the same set of cells but the set that shows in landscape should overlap the set shown in portrait as much as possible. As you say Photos app accomplishes this. Showing a completely different set however is not good behavior even if Apple's classes were to do so.

Interestingly enough the problem does not manifest at all with the current AQGridView code when you rotate the device while viewing cells close to the top of the table and the conditions exist (ie; different number of cells per row in a large table).

Thanks again.

On Mon, Sep 17, 2012 at 2:15 PM, Evadne Wu notifications@github.com wrote:

Sorry — my fault in not communicating clearly. Showing some other cells, while at the same time maintaining offset, is certainly bad user experience and will nevertheless require code either inside or outside the project to handle. The original goals of the project is to match NSCollectionView as much as possible. It conflicts with optimizing for developer happiness.

For a very quick testing, iBooks exhibits the same behavior on device rotation — it shows a different set of cells. The Photos app on iPad handles this case better, but I fail to find any app where, after rotating to and back from a certain orientation, the grid view shows the same set of cells. For example, go to Photos.app, open the Photo Stream, go landscape, navigate to the last row and scroll one row downwards. Now rotate to portrait. The app should show the last photo on the last row. Rotate back to landscape and the grid view now shows the last cell as well.

Judging from code and how the sample app runs, we’ll get the behavior that Photos.app show in AQGridView.

As an additional example why a simple fit-to-bottom or fit-to-displayed-index-paths will sometimes fail, enable the Emoji keyboard on the iPhone, then go to any app that supports landscape input. Scroll to the last page in the segment, then start rotating the device to landscape and back. You end up on the first page of the segment.

The desired behavior is not consistently handled even across iOS apps and it’s important for it to be well-defined. However, it’s always okay for AQGridView to have its own idiosyncrasy — as long as it is very reasonably documented — and I surely have made a judgmental mistake of rejecting a patch for a certain defect.

I’d like to pull this into develop shortly, test on demo, internal and public projects, and wait for community feedback.

Thanks again for your feedback.

— Reply to this email directly or view it on GitHubhttps://github.com/AlanQuatermain/AQGridView/pull/156#issuecomment-8625073.