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

ANDROID-14001 Fix chevron glitch on button link #318

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

yamal-alm
Copy link
Contributor

@yamal-alm yamal-alm commented Nov 16, 2023

🥅 What's the goal?

There is a UI glitch when a button2 with "link" style and chevron is rendered because of how chevron height is calculated. See the following video:

Grabacion.de.pantalla.2023-11-16.a.las.9.11.11.mov

🚧 How do we do it?

Use built in IntrinsicSize.Min as the chevron height instead of listening to the height of the text which makes the chevron to switch from big to small very quickly.

☑️ Checks

  • I updated the documentation, including readmes and wikis. If this is a breaking change, update UPGRADING.md to inform users how to proceed. If no updates are necessary, indicate so.
  • Tested with dark mode.
  • Tested with API 23.

🧪 How can I test this?

Grabacion.de.pantalla.2023-11-16.a.las.9.30.03.mov

Comment on lines +180 to +184
Box(
modifier = Modifier
.height(IntrinsicSize.Min)
.aspectRatio(CHEVRON_ASPECT_RATIO)
.align(Alignment.CenterVertically)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to wrap the whole CompositionLocalProvider element inside a box because it doesn't allow any modifier and IntrinsicSize only applies to direct children of Row

@yamal-alm yamal-alm marked this pull request as ready for review November 16, 2023 08:49
Copy link

📱 New catalog for testing generated: Download

Copy link
Member

@dagonco dagonco left a comment

Choose a reason for hiding this comment

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

🪄

Copy link
Contributor

@DevPabloGarcia DevPabloGarcia left a comment

Choose a reason for hiding this comment

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

Thanks 😄

@yamal-alm yamal-alm merged commit f8a2bc1 into main Nov 16, 2023
4 of 5 checks passed
@yamal-alm yamal-alm deleted the button-link-with-chevron-glitch branch November 16, 2023 09: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.

3 participants