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

Fix Select Location headline size #5148

Merged

Conversation

Rawa
Copy link
Contributor

@Rawa Rawa commented Sep 12, 2023

Quick fix for headline size to look the same as rest of app bars. Right now the font size is hardcoded for the top bar and not in the compose theme so once we move away from the CollapsingTopBar we should update all these fonts.


This change is Reviewable

@Rawa Rawa added the Android Issues related to Android label Sep 12, 2023
@Rawa Rawa self-assigned this Sep 12, 2023
@linear
Copy link

linear bot commented Sep 12, 2023

DROID-318 Fix AppBar title size in Select Location screen

Title is too small, should be bigger according to design. Possibly wrong in design

https://app.zeplin.io/project/5f928a32fdc9962af9018d70/screen/63f60797fa4c8443c8561df5

@Rawa Rawa requested a review from Pururun September 12, 2023 10:59
Copy link
Collaborator

@albin-mullvad albin-mullvad left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 108 at r1 (raw file):

                        .padding(end = Dimens.titleIconSize),
                textAlign = TextAlign.Center,
                style = MaterialTheme.typography.headlineSmall.copy(fontSize = 20.sp),

Just to double check, will this be reflected in the design documents as well?

Copy link
Contributor

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)

@Rawa
Copy link
Contributor Author

Rawa commented Sep 18, 2023

@albin-mullvad Regarding font size. I've aligned with design. All our collapsible have the wrong lower end text size (20sp should be 15sp). We've agreed on that for now we set it to 20sp on this screen and then replace it along with all the top bar issues when we replace the entire top bar.

Copy link
Collaborator

@albin-mullvad albin-mullvad left a comment

Choose a reason for hiding this comment

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

Sounds good! 👍

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Pururun Pururun force-pushed the fix-appbar-title-size-in-select-location-screen-droid-318 branch from 34a771b to 8ed6565 Compare September 18, 2023 09:45
@Pururun Pururun merged commit a91c5d9 into main Sep 18, 2023
12 checks passed
@Pururun Pururun deleted the fix-appbar-title-size-in-select-location-screen-droid-318 branch September 18, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants