Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show on App Launch: Fix option item height #5209

Conversation

mikescamell
Copy link
Contributor

@mikescamell mikescamell commented Oct 29, 2024

Task/Issue URL: https://app.asana.com/0/1207908166761516/1208648215243996/f

Description

Fixes the height of the options on the Show on App Launch screen to 48dp. As a result of this we should do a follow up to add the possibility to our OneLine and TwoLineListItems to allow for a RadioButton

This was done with minimal code changes to reduce any effect on the VPN country selection list.

Steps to test this PR

Show on App Launch

  • Open Show on App Launch
  • Check items are 48dp high

VPN Location List

  • Open VPN Location screen
  • Check items are unchanged and remain 64dp high

UI changes

Before After
Screenshot_20241029_180839 Screenshot_20241029_180918
Screenshot_20241029_181258 Screenshot_20241029_181003

The minHeight of the RadioButton was stopping the item shrinking to it's minHeight of 48dp when it was eligible to do so
This was the easiest way to allow 48dp for ShowOnAppLaunch and 64dp for VPN countries which we do in the next commit
Now that we can actually shrink to 48dp we need to ensure that the vpn country items match their existing height of 64dp as a result

This was the least intrusive way to fix these issues before following up later with additions to OneLineListItem and TwoLineListItem
Copy link
Contributor Author

mikescamell commented Oct 29, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mikescamell and the rest of your teammates on Graphite Graphite

@mikescamell mikescamell marked this pull request as ready for review October 29, 2024 18:30
Copy link
Contributor

@anikiki anikiki left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mikescamell mikescamell merged commit e4a53b2 into feature/mike/show-on-app-launch/implementation Oct 30, 2024
6 of 7 checks passed
@mikescamell mikescamell deleted the feature/mike/show-on-app-launch/fix-radioitemlist-padding branch October 30, 2024 11:34
@mikescamell mikescamell restored the feature/mike/show-on-app-launch/fix-radioitemlist-padding branch October 30, 2024 11:45
@mikescamell mikescamell deleted the feature/mike/show-on-app-launch/fix-radioitemlist-padding branch January 10, 2025 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants