Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move top and bottom bars to the sides for compact heights. #182

Closed
wants to merge 2 commits into from

Conversation

vickeryj
Copy link
Contributor

When faced with a compact height size class (landscape mode
in phones) the bottom bar blocks a large portion of the screen
and makes it very difficult to take pictures.

This commit detects the current vertical size class, and if it
is compact, moves the bottom and top bars to the side, which is
in line with the way the iPhone's camera app works

img_0564

When faced with a compact height size class (landscape mode
in phones) the bottom bar blocks a large portion of the screen
and makes it very difficult to take pictures.

This commit detects the current vertical size class, and if it
is compact, moves the bottom and top bars to the size, which is
in line with the way the iPhone's camera app works
@mention-bot
Copy link

@vickeryj, thanks for your PR! By analyzing the annotation information on this pull request, we identified @RamonGilabert and @zenangst to be potential reviewers

@richardtop richardtop mentioned this pull request Jul 24, 2016
@onmyway133
Copy link
Contributor

@vickeryj this is great. But how about the gallery and the gesture to drag the gallery?

@vickeryj
Copy link
Contributor Author

Doh! I should have mentioned that in the PR. I disabled the gallery in compact height mode because it felt like the primary use case for landscape mode was taking pictures, and the gallery in landscape mode was somewhat awkward to use.

The gallery could be restored, either opening in a vertical orientation, or sliding up from the bottom. I actually prefer to have it disabled, but if you think it's important to have it for this PR to get merged I can find some time to implement it, probably with an option to disable the gallery in compact height mode.

@onmyway133
Copy link
Contributor

@vickeryj I'm OK with your approach. My only concern is if we support gallery in landscape, then the scrolling, images and check indicator should look good in landscape also

multiplier: 1, constant: 0))
}

addConstraint(NSLayoutConstraint(item: doneButton, attribute: .CenterX,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better way is to define 2 sets of constraints, and toggle their active accordingly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I ran into with that approach is storage of references to the constraints. Since all these constraints are added in an extension, I don't have access to stored properties. I could put them in global state, but I figured adding and removing constraints this way was better than this option.

Another option would be to re-factor this code out of an extension and into the class implementations, allowing me to store references to the constraints in properties.

What do you think of those options? Do have a suggestion for a different option?

@onmyway133 onmyway133 mentioned this pull request Jul 26, 2016
@RamonGilabert
Copy link
Contributor

Closing for #186.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants