fix(Android): hide onboarding images that are likely to collide with text #770
Conversation
| Modifier.fillMaxWidth() | ||
| .height(52.dp) | ||
| .padding(start = 32.dp, end = 32.dp), | ||
| modifier = buttonModifier.padding(start = 32.dp, end = 32.dp), |
There was a problem hiding this comment.
Do we still want the 32 padding on all the buttons if that was the thing causing the line breaks that messed up formatting? 16 seems reasonable and safer, but not a huge deal either way, these all need to just be their own composable, which we shouldn't spend time on now 🤷
There was a problem hiding this comment.
I guess we don't; 16 across all buttons might be the better move there.
There was a problem hiding this comment.
Yeah I considered that, but wasn't sure if it was ok to be adding back some of the padding around the Text that was removed in #754 for the elevator accessibility buttons (not sure why just those had the padding on the text rather than the button, maybe that was part of the problem).
There was a problem hiding this comment.
Actually, no, the weird inconsistencies between these buttons are annoying to disentangle here, and the right fix is a more comprehensive refactor anyway. The one that's most likely to wrap is "Show elevator closures", and that one's getting its padding removed in #754 anyway.
Summary
Ticket: 🤖 | Fix dynamic text in onboarding
Given the variety of screen sizes on Android, these thresholds may only be correct at the size Android Studio rendered the preview at while I was working on this, and a more robust solution would be to use the layout subsystem that I don't really understand to actually use the size of the text to show or hide the images, but in the interest of shipping in the short term, this seems like it's better than nothing, which is the main objective.
I'm also opportunistically fixing the bug where the button heights are fixed at 52 and so a second line of text will get cut off - on iOS that's a minimum height, and on Android it seems like it should be too.
iOS
[ ] If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?[ ] Add temporary machine translations, marked "Needs Review"android
[ ] All user-facing strings added to strings resource in alphabetical order[ ] Expensive calculations are run inwithContext(Dispatchers.Default)where possibleTesting
Checked in the Android Studio preview (which is the easiest place to apply arbitrary text scale values) that the images are hidden before the text clips into them.