WeTransfer / WeScan

Document Scanning Made Easy for iOS
MIT License
2.85k stars 555 forks source link

iPhone X layout issues #92

Closed askeland closed 5 years ago

askeland commented 5 years ago

Looks like the changes in #83 did not work fully as expected. On iPhone X the flash icon is vertically aligned center and the Auto/manual button is vertically aligned in the bottom. Not sure which one of them that is most corrrect, but... Added a screenshot:

img_ead76fef4674-1

jcampbell05 commented 5 years ago

May need to account for safe area

askeland commented 5 years ago

They account for the safe area, but I believe that the UIToolbar is not optimal for this. You see a lot of users with same issue on SO and UIToolbar does not like custom heights. It might be better to just use an UIView as “toolbar” and UIButtons inside instead.

jcampbell05 commented 5 years ago

@askeland yes I think thats the best solution since I'm not sure if we really need a UIToolbar

julianschiavo commented 5 years ago

Well, someone can open a PR for that but it’s gonna be really annoying as you can’t use UIBarButtonItems or the auto blurred appearance :/

askeland commented 5 years ago

@justJS What was the main motivation for going away from just keeping it in a UINavigationBar?

julianschiavo commented 5 years ago

🤔 I don’t think we ever considered it, before the toolbar it was the default nav bar but flash/auto scan didn’t exist. Wouldn’t It not have the same style tho? I’ll check it out later when I’m on my computer.

jcampbell05 commented 5 years ago

Blurred there are UIVisualEffectViews :) so you don't need a UIToolbar for that

julianschiavo commented 5 years ago

I know, but you probably can't add one to a nav bar? Haven't checked yet

jcampbell05 commented 5 years ago

@justJS no need the navbar handles that :)

jcampbell05 commented 5 years ago

You can also instantiate a UINaviagationBar

julianschiavo commented 5 years ago

I did as suggested in #94 (manually creating blur view to make it look more like Camera app). Thanks for the suggestions @jcampbell05 @askeland. 🙂

julianschiavo commented 5 years ago

Here's a screenshot 😛 simulator screen shot - iphone xs - 2018-11-30 at 19 02 21

AvdLee commented 5 years ago

@justJS is this one fixed? If so, you can close it :)

julianschiavo commented 5 years ago

Fixed in #94