-
Notifications
You must be signed in to change notification settings - Fork 0
Fix ScoreBox UI #48
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 ScoreBox UI #48
Conversation
zachseidner1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this Amy. I think there are some code quality improvements you could make for the UI. Additionally, the main thing this was supposed to fix is how long scores appear, which your change does not address. See how the preview I added displays differently than the Figma? I think you'll need a minimum width on the text, and also account for text overflow and use ellipses
app/src/main/java/com/cornellappdev/score/components/ScoreBox.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/cornellappdev/score/components/ScoreBox.kt
Outdated
Show resolved
Hide resolved
… Amy/fix-scoring-box-summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just barely ready for MVP but there are still some issues that I'd like you to resolve.

We're missing the drop shadow around the score box. The border gets cut off at the corner. Also, the school names should have a min width of 60 as specified by the design. Also, the numbers for the periods are not center aligned with their scores.
Just merge it for now, and I'll create a GitHub issue so we can follow up later.
Overview
Address issue #44
Changes Made
Rounded corners and included border
Test Coverage
Checked visually, matches Figma