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 [email protected] 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//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 [email protected] 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//pull/156#issuecomment-8625073.