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

bug-34 :: Fix coin name overflow in coin detail header #82

Merged
merged 3 commits into from
Jul 13, 2023

Conversation

drahovac
Copy link
Contributor

Suggestion to fix #34

It is maybe overkill to use Layout for measurement, but I wanted to add min width for name part, so even if balance part was too long (small display, big font, large amount), the view would not overflow. If we do not care for edge case, that amount part is too long for the screen, just weight modifier can be used.

@drahovac

This comment was marked as outdated.

@RyanKoech
Copy link
Owner

What I had in mind concerning this was:

  • Basic Fix: Using ellipses. Specifically for the name. The price will always have weight of 1 since anything value at that very high amount is an extreme edge case. (Trillions of dollars for a coin? 🤣)
  • Cool Fix : Having the name get cut off but its animates horizontally to show the whole name

Ultimately we don't need to worry about an insanely large price.

@drahovac drahovac force-pushed the bug/34_long_coin_name_overflow branch from 22d824b to c774c42 Compare July 13, 2023 17:27
@drahovac
Copy link
Contributor Author

Higher version of Compose has a modifier called "basicMarquee" that animates text horizontally. I needed to update the Compose version (as well as some other versions for compatibility). The issue has been fixed in "coin_detail" and "coin_list."

@drahovac
Copy link
Contributor Author

Screenshot_1689268983
Screenshot_1689268986
Screenshot_1689268989

@drahovac drahovac marked this pull request as ready for review July 13, 2023 17:36
@RyanKoech
Copy link
Owner

Awesome! Looks all good!

@RyanKoech
Copy link
Owner

One more thing. You can bump the versionPatch in dependencies.gradle up by one then it'll be all good to merge.

@RyanKoech RyanKoech merged commit d79fef6 into RyanKoech:dev Jul 13, 2023
2 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.

Fix long coin name over flow.
2 participants