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

🔧 This addresses the case where the image of the ProfileCard used for sharing on SNS etc. is corrupted in some cases. #928

Conversation

Corvus400
Copy link
Contributor

@Corvus400 Corvus400 commented Sep 2, 2024

Issue

  • None.

Overview (Required)

  • Using the CompositionLocalProvider, we fixed the Density and FontScale for FlipCards used for shooting.
  • Until this was fixed, the text size would fluctuate as reported in the issue and as shown in the screenshot, and in some cases the cards would be displayed squashed.
  • With this fix, the appearance should now be consistent on all devices.

Links

Screenshot (Optional if screenshot test is present or unrelated to UI)

Before Portrait Minimum SmallPhone After Portrait Minimum SmallPhone
Before Landscape Minimum SmallPhone After Landscape Minimum SmallPhone
Before Portrait Max SmallPhone After Portrait Max SmallPhone
Before Landscape Max SmallPhone After Landscape Max SmallPhone

Before Portrait Minimum Pixel 7 Pro After Portrait Minimum Pixel 7 Pro
Before Landscape Minimum Pixel 7 Pro After Landscape Minimum Pixel 7 Pro
Before Portrait Max Pixel 7 Pro After Portrait Max Pixel 7 Pro
Before Landscape Max Pixel 7 Pro After Landscape Max Pixel 7 Pro

… sharing on SNS etc. is corrupted in some cases.

This was addressed by using the CompositionLocalProvider to ensure that the composable for taking photos has a fixed density and font scale.
@github-actions github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 07:03 Inactive
Copy link

github-actions bot commented Sep 2, 2024

Detekt check failed. Please run ./gradlew detekt --auto-correct to fix the issues.

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 07:08 Inactive
…rectly, so I applied the same fix as the others.
@github-actions github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 07:24 Inactive
@Corvus400 Corvus400 marked this pull request as ready for review September 2, 2024 07:39
Comment on lines +64 to +69
CompositionLocalProvider(
LocalDensity provides Density(
density = 1f,
fontScale = 1f,
),
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If fontScale is not set to a fixed value, the text will either overflow or become smaller if the device settings are changed.
So, I used the CompositionLocalProvider to fix the density and font scale of the composable for taking photos so that the display would not break.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have never thought of this before. This is super smart 👀

@@ -108,7 +108,7 @@ private fun ShareableCardContent(
)
.background(LocalProfileCardTheme.current.primaryColor),
) {
Box(modifier = Modifier.padding(vertical = 30.dp)) {
Box(modifier = Modifier.padding(vertical = with(density) { verticalPaddingPx.toDp() })) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part has been forgotten to be modified, and is one of the causes of the display corruption when in landscape mode.

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 18:01 Inactive
Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

Thank you for finding and fixing your contribution!

@takahirom takahirom merged commit 7896b10 into DroidKaigi:main Sep 3, 2024
6 checks passed
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.

3 participants