enormego / PhotoViewer

Quick PhotoViewer for the iPhone. Built upon our other reliable libraries: EGOImageLoading and EGOCache.
http://developers.enormego.com
660 stars 141 forks source link

iPad version - completely broken in popOvers #8

Open wuf810 opened 13 years ago

wuf810 commented 13 years ago

Number of issues:

Scrolling images on the iPad in popOvers is completely broken. It may work if all images are the same size but won't currently work with the way you calculate the centre of the image being scrolled to as the calculation is based on a fixed sized images.

Things are further aggravated by the copious amounts of hardcoded assumptions when using on an iPad. For example just because images are being shown in popOvers doesn't mean they need to be auto resized. It may be helpful to expose a few more of these private properties too.

devindoty commented 13 years ago

I pushed the latest version, which should take care of those issues. If not please let me know.

Devin

wuf810 commented 13 years ago

Devin, I'll try them out. Thank you very much…I struggled trying to fix myself and can't easily explain how much I appreciate this and your efforts.

I'll let you know how I get on.

wuf810 commented 13 years ago

Hi Devin

Been testing out the new version. Everything is OK if the PhotoViewer is presented in a brand new popOver.

The issue I have is that I already have a popOver on screen and this allow the user to click a photo thumbnail.

I then define the source, create the PhotoViewer, resize the popOver content and then push it on an existing navigationController using pushViewController.

To all intents and purpose the PhotoViewer appears and looks OK (it is in a popOver beit created earlier). However when you start to drag images back and forth or even use the navigation arrows , it appears the scroll view's width or position calculation per image is all over the place.

I hope this makes sense. So basically to test, create a popOver and then try pushing the PhotoViewer onto it to see what happens.

Thanks, M

wuf810 commented 13 years ago

I think the issue is that this photoviewer does not know it is actually being displayed in a popOver…

devindoty commented 13 years ago

Yep you're definitely right. It should have been checking the navigation controller view hierarchy for the popover. The latest revision should fix that :)

wuf810 commented 13 years ago

I shall try it now…thank you :-)

wuf810 commented 13 years ago

Devin, well I think you're absolutely brilliant! It is working.

However I have noticed a couple of issues:

1) If I set the photoController.contentSizeForViewInPopover to something larger than the popOverController that originally calls the photoViewer, then the sizing issue re-occurs; the first image is incorrectly sized and you can drag scroll beyond the last image

I can live with this personally but just in case there is an obvious fix.

2) Is there anyway to get the Navigation buttons in the popOver view? Obviously they appear in full screen, but just wondering if there is a property to set.

Either way thank you again. really appreciate your efforts and help with this!

devindoty commented 13 years ago

Thanks!

Both of those should be fixed with the revision I just pushed. In regards to the first one, I added an observer to the photo view controllers contentSizeForViewInPopover:, if you change contentSizeForViewInPopover: just make sure it's after the controller is in the nav stack.

For example: [self.navigationController pushViewController:photoController animated:YES]; photoController.contentSizeForViewInPopover = CGSizeMake(400.0f, 400.0f);

wuf810 commented 13 years ago

Hi Devin,

Just tried the latest version.

1) Sizing seems to be working perfectly now. Thank you.

A note to any one else reading this, it you call movetoPhotoIndex before the view appears, makes sure it comes after any code setting the contentSizeForView. For Example

[self.navigationController pushViewController:photoController animated:YES]; photoController.contentSizeForViewInPopover = CGSizeMake(640.0f,480.0f); [photoController moveToPhotoAtIndex:selectedImageCounter animated:NO];

2) The navigation buttons are back! However is there a way to add the "show full screen" button at the same time or is it either or? It would be great to give user option to jump to full screen mode from the popOver.

Thanks again, Michael.

wuf810 commented 13 years ago

Re point 2 above the fullsize toggle button not showing,

I found a bug.

Line 353 EGOPhotoViewController:

UIBarButtonItem *scaleButton = [[UIBarButtonItem alloc] initWithImage:[UIImage imageNamed:@"egopv_fullscreen_button.png.png"] style:UIBarButtonItemStyleBordered target:self action:@selector(toggleFullScreen:)];

has an extra ".png" suffix in the image name. Should be:

UIBarButtonItem *scaleButton = [[UIBarButtonItem alloc] initWithImage:[UIImage imageNamed:@"egopv_fullscreen_button.png"] style:UIBarButtonItemStyleBordered target:self action:@selector(toggleFullScreen:)];

Finally I fixed something myself :-)

devindoty commented 13 years ago

good catch, thanks!