search

Automattic / stories-android

Loop concept app - WP Stories library
GNU General Public License v2.0
20 stars 6 forks source link

Fix color picker bottom sheet clipping #656

Closed aforcier closed 3 years ago

aforcier commented 3 years ago

Fixes #597.

Since I'm now able to consistently reproduce the issue on a Pixel 5, I did some debugging to try and sort it out. I tried a few approaches but ultimately the extremely simple one in this PR seems best.

Essentially what was happening was that the minimum height for the bottom sheet was just a little too short, but for most devices the keyboard was short enough that we'd match the bottom sheet height to the keyboard height anyway, and that was taller than the minimum height.

The keyboard height on the Pixel 5 though is just tall for this check to be false, so the default height is used, and it's a bit too short.

After this adjustment the bottom sheet will match the keyboard height on a Pixel 5 too. There might be a bit more padding at the bottom than ideal, but in practice having the bottom sheet match the keyboard height makes for a much less janky transition so IMO it's worth it.

Here's the before/after on a Pixel 5:

pixel-5-beforeafter

Checked that nothing's changed on the Pixel 3:

pixel-3-beforeafter

I also checked with @bjtitus and confirmed this fix resolves the issue on his Samsung Galaxy S10 Lite (the source of the original issue report) 🎉

Side note: I thought the issue might be related to https://github.com/wordpress-mobile/WordPress-Android/issues/12491, which I also see consistently on the Pixel 5 - but given what the problem with the color picker ended up being I'm not so sure.

To test

Run the demo app on a few different devices, bring up the text color picker, and make sure it's not clipped/behaves well everywhere.

peril-automattic[bot] commented 3 years ago

You can test the changes on this Pull Request by downloading the APK here.