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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MediaPicker xib layout fix #1372

Closed
wants to merge 1 commit into from
Closed

MediaPicker xib layout fix #1372

wants to merge 1 commit into from

Conversation

automactic
Copy link

@automactic automactic commented Jul 4, 2017

Currently, the CaptureView in the MediaPickerController has a fixed aspect ratio. This results in not being able to see the CaptureButton without scrolling on iPads. In this pull request, I am trying to fix this issue by removing the CaptureView's aspect ratio constrain and adding CaptureView's height equals MainScrollView's parent view height constrain.

I have not tested this on real device since I cannot build this app on my machine (profiling issue). Please help me test it if anyone could. Thanks!

BTW, awesome app! 馃槉

@giomfo giomfo self-requested a review July 6, 2017 12:30
giomfo added a commit that referenced this pull request Jul 11, 2017
The camera preview was set up with a wrong frame value. We wait for the first `viewDidLayoutSubviews` call before setting up the preview.
#686

+ fix the wrong preview layout on iPad described in PR #1372 MediaPicker xib layout fix.
@giomfo
Copy link
Member

giomfo commented Jul 11, 2017

@automactic: First of all, thanks for your contribution. I reviewed your PR, I keep the idea of adding the constraint on the container height. But I did not remove the aspect ratio constraint (I reduced its priority compared to the height constraint).

Because I developed/tested these changes on my own branch, and fixed another issue (#686). I created my own PR(#1388) to merge on develop. I will close this one.

BTW, if you want to submit other PRs, you should prepare them to commit into vector-im:develop (and not master). And you will have to sign off them (see details in https://github.com/vector-im/riot-ios/blob/master/CONTRIBUTING.rst).

@giomfo giomfo closed this Jul 11, 2017
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.

None yet

2 participants