wordpress-mobile / WordPress-Editor-iOS

⛔️ [DEPRECATED] A reusable iOS rich text editor component.
GNU General Public License v2.0
1.06k stars 210 forks source link

Fix iphone x issues #899

Closed SergioEstevao closed 6 years ago

SergioEstevao commented 6 years ago

This PR solves layout issues on the iPhoneX for the legacy editor and the hybrid editor.

How to test:

frosty commented 6 years ago

Just running the demo app – the positioning of items looks better, but it seems like some of them are still getting squashed in portrait orientation:

screen shot 2017-10-09 at 14 53 42

This is on iPhone 8, iOS 11.0

bummytime commented 6 years ago

@SergioEstevao positioning does look good, but I am seeing a similar issue with the media button on the iPhone 8 (iPhone X looks good):

screen shot 2017-10-09 at 9 31 09 am
<UIImageView: 0x7fae44135750; frame = (0 0; 35 44); clipsToBounds = YES; opaque = NO; userInteractionEnabled = NO; 
frosty commented 6 years ago

fwiw, seeing the same on the legacy editor – here's one of the buttons:

<UIImageView: 0x7ff817f3f030; frame = (0 0; 39 44); clipsToBounds = YES; opaque = NO; userInteractionEnabled = NO; layer = <CALayer: 0x60800043cc00>>

SergioEstevao commented 6 years ago

@frosty @bummytime any ideas to fix this? it looks that iOS11 we don't have full control of the size of the buttons. They are changed depending of the space available.

SergioEstevao commented 6 years ago

@bummytime and @frosty ready for another look.

frosty commented 6 years ago

Latest changes definitely fix the 'flying toolbar', and the sizes of the items are more consistent. However, I noticed on an iPhone SE sized device, we lose the last toolbar item on iOS 11, which doesn't happen on iOS 10:

editor-bar-10-11

SergioEstevao commented 6 years ago

@frosty ready for another look.

frosty commented 6 years ago

Same on iPhone 8 – notice the small image icon:

screen shot 2017-10-10 at 14 20 25
SergioEstevao commented 6 years ago

@frosty tweaked it a bit more. Have a look.

bummytime commented 6 years ago

@SergioEstevao The icon sizes look good and all display across multiple devices.

However, I noticed there was extra padding on the bottom of the format bar when the keyboard first displays (this happened on both iOS 10 & 11, multiple devices):

format_bar

The padding disappears when you dismiss the soft keyboard and then re-enable it.

SergioEstevao commented 6 years ago

@bummytime and @frosty ready for a final look

bummytime commented 6 years ago

@SergioEstevao really nice job keeping at this!

On the iPhone X, when the soft keyboard is not visible, the format bar is not tap-able:

iphone_x_11

This does not appear to be the case on iPads with a hardware keyboard (even when the shortcuts & predictive text bars are turned off). Sooooo, I am not sure if this is a showstopper or not, but I thought I would point it out.

bummytime commented 6 years ago

Also, the bottom inset fix from your latest commit seems to have fixed that issue on both iOS 10 & 11! 😄

SergioEstevao commented 6 years ago

@bummytime thanks for keep testing it. So I did one more change that solves the problem you found out. Can you please give it a final look?

@frosty if you want to check it too?

bummytime commented 6 years ago

@SergioEstevao Your latest commit fixed all of the issues seen on the X (plus it still works everywhere else). 🎉 After addressing @frosty's comments above it's a :shipit: from me.

SergioEstevao commented 6 years ago

@frosty changed the toolbar height to a constant. Ready for final approval!