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 copy button scaling issue #4951

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

sabercodic
Copy link
Contributor

@sabercodic sabercodic commented Aug 2, 2023

Fixed: DROID-240, DROID-241


This change is Reviewable

@sabercodic sabercodic added the Android Issues related to Android label Aug 2, 2023
@linear
Copy link

linear bot commented Aug 2, 2023

DROID-240 Copy button scaling issues

The following is from a OnePlus 6T device:

Screenshot_20230801-173558.jpg

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.

This results in just 12 dots being shown on my OnePlus 6T, which is not the intended behavior. I believe the number/dots should be scaled down if they don't fit.

Reviewable status: 0 of 3 files reviewed, all discussions resolved

@sabercodic sabercodic force-pushed the copy-button-scaling-issues-droid-240 branch 2 times, most recently from 5879ccd to 07ab937 Compare August 3, 2023 08:20
@linear
Copy link

linear bot commented Aug 4, 2023

DROID-241 Unexpected large spinner when resuming the account view

See the below screenshot showing the large spinner:

Screenshot_20230802-155623.jpg

@sabercodic sabercodic force-pushed the copy-button-scaling-issues-droid-240 branch 3 times, most recently from 8d4481e to dcddce5 Compare August 4, 2023 13:19
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.

Reviewed 1 of 3 files at r1, 1 of 2 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @sabercodic)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 73 at r3 (raw file):

    maxLines: Int = Int.MAX_VALUE,
) {
    var fontSizeValue by remember { mutableStateOf(maxTextSize.value) }

Change to adjustedFontSize

Code quote:

fontSizeValue

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 74 at r3 (raw file):

) {
    var fontSizeValue by remember { mutableStateOf(maxTextSize.value) }
    var readyToDraw by remember { mutableStateOf(false) }

Change to isReadyToDraw

Code quote:

readyToDraw

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 82 at r3 (raw file):

        fontSize = fontSizeValue.sp,
        onTextLayout = {
            if (it.didOverflowHeight && !readyToDraw) {

Change to .not()

Code quote:

!

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 83 at r3 (raw file):

        onTextLayout = {
            if (it.didOverflowHeight && !readyToDraw) {
                val nextFontSizeValue = fontSizeValue - DEFAULT_TEXT_STEP.value

This should be a function argument

Code quote:

DEFAULT_TEXT_STEP.value

@sabercodic sabercodic force-pushed the copy-button-scaling-issues-droid-240 branch from dcddce5 to a8d463f Compare August 4, 2023 15:30
Copy link
Contributor Author

@sabercodic sabercodic left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @albin-mullvad)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 73 at r3 (raw file):

Previously, albin-mullvad wrote…

Change to adjustedFontSize

Done.


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 74 at r3 (raw file):

Previously, albin-mullvad wrote…

Change to isReadyToDraw

Done.


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 82 at r3 (raw file):

Previously, albin-mullvad wrote…

Change to .not()

Done.


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Text.kt line 83 at r3 (raw file):

Previously, albin-mullvad wrote…

This should be a function argument

Done.

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 r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@sabercodic sabercodic force-pushed the copy-button-scaling-issues-droid-240 branch from a8d463f to 4dc6daf Compare August 7, 2023 11:16
@albin-mullvad albin-mullvad merged commit d3a7629 into main Aug 7, 2023
9 checks passed
@albin-mullvad albin-mullvad deleted the copy-button-scaling-issues-droid-240 branch August 7, 2023 11:19
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.

2 participants