Skip to content
This repository has been archived by the owner on Jun 16, 2022. It is now read-only.

LL-451 Bottom modal glitches sometimes on certain Android phones #617

Merged
merged 11 commits into from
Dec 19, 2018

Conversation

MortalKastor
Copy link
Contributor

@MortalKastor MortalKastor commented Dec 7, 2018

On some Android phones, under some conditions, the bottom modal shows a ugly glitch:

before

This PR change that to something less ugly, but still suboptimal:

android screencap 2018-12-05 18 45 12

It's not a real fix, but:

  • It's an edge case that happens infrequently on a limited list of devices in a specific (not default (?)) configuration
  • It doesn't break the app
  • This PR shouldn't bring regressions for the cases which already weren't glitchy
  • It's less ugly

Also, as a bonus, this PR fixes facebook/react-native#4934.
This could fix the weird statusbar glitch on Android where there's too much top padding under some undetermined conditions.
Maybe.
I don't know.
It's Friday.

Type

Bug Fix(ish)

Context

https://ledgerhq.atlassian.net/browse/LL-451

Parts of the app affected / Test plan

On Galaxy S9, set the navigation bar to self hide then spam a modal until its bottom margin is too tall rather than showing the underlying UI.

src/components/BottomModal.js Outdated Show resolved Hide resolved
@gre gre added the HODL label Dec 7, 2018
@gre
Copy link
Contributor

gre commented Dec 7, 2018

i'm setting a HODL because we'll try to merge after apple submission (not sure about risk of modal lib bump and this only concerns Android)

Copy link
Contributor

@Arnaud97234 Arnaud97234 left a comment

Choose a reason for hiding this comment

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

Solved on android but some regression has been inserted :
ios
iphone 8
Trade safely drawer.

How to reproduce
Reset Ledger Live.
Complete onboarding process.

img_8222

Android
Android 8 (Galaxy S9)
Trade safely modal

Only the top of the "Got it" button is clickable

gre
gre previously approved these changes Dec 13, 2018
@gre gre removed the HODL label Dec 13, 2018
Copy link
Contributor

@Arnaud97234 Arnaud97234 left a comment

Choose a reason for hiding this comment

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

Some drawer still have the bug on Android 9, Galaxy S9.
See comment in Jira : https://ledgerhq.atlassian.net/browse/LL-451

@meriadec
Copy link
Member

wonder if it's really blocking if the bug is only remaining on 1 device. wdyt @gre

@gre
Copy link
Contributor

gre commented Dec 13, 2018

If it's a regression on that device then I feel yes it's blocking. Better have a functional button with a glitchy UI (develop) rather than a button not reachable

@MortalKastor
Copy link
Contributor Author

(the un-reachable button regression has been fixed, btw)

@gre gre requested a review from Arnaud97234 December 18, 2018 20:08
@MortalKastor MortalKastor dismissed Arnaud97234’s stale review December 18, 2018 20:09

Should be fixed for good this time

@MortalKastor MortalKastor merged commit 572b820 into LedgerHQ:develop Dec 19, 2018
@MortalKastor MortalKastor deleted the LL-451 branch December 19, 2018 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dimensions.get('window').height is sometimes wrong on Android
4 participants