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.

Sign up for GitHub

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

Jump to bottom

Redesign play page UI #47

Merged
merged 8 commits into from
Jun 8, 2023
Merged

Redesign play page UI #47

merged 8 commits into from
Jun 8, 2023

Conversation

mfletcher2
Copy link
Contributor

@mfletcher2 mfletcher2 commented Jun 3, 2023 •
edited
Loading

This PR changes the components of PlayPage such that:

  • Large images are replaced with emojis
  • Text is centered
  • Buy/Sell buttons are different colors
  • A progress bar representing cow health is added
  • The profits table headers are more intuitive
  • The profits table are displayed newest first by default
  • A percent sign is shown in the table for health

Screenshots

Before:
Screenshot_20230602_211955
After:
Screenshot_20230602_212644
(Your emojis will probably look different)

Storybook

Before

ManageCows
FarmStats
ProfitsTable

After

ManageCows
FarmStats
ProfitsTable

Closes #39

@mfletcher2 mfletcher2 added FIXME-Needs Peer CR please get a team member CR then remove this label s23-6pm-1 labels Jun 3, 2023
@TheRealJBomb
Copy link
Contributor

image

Note, you are not up-to-date with main. A fix was applied to fix this table overflow problem.

@TheRealJBomb TheRealJBomb added FIXME-Respond to CR Please review the comments in the code review and address them; then remove this label and removed FIXME-Needs Peer CR please get a team member CR then remove this label labels Jun 5, 2023
@TheRealJBomb
Copy link
Contributor

Also, we don't want your changes to be overwritten by what Alex is doing in #29. You both may want to communicate with each other and add his additions into this PR.

@mfletcher2 mfletcher2 added FIXME-Needs Peer CR please get a team member CR then remove this label NEEDS ATTENTION and removed FIXME-Respond to CR Please review the comments in the code review and address them; then remove this label NEEDS ATTENTION labels Jun 6, 2023
JoeIsNoJoe
JoeIsNoJoe approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@JoeIsNoJoe JoeIsNoJoe left a comment

Choose a reason for hiding this comment

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

LGTM

@albertyu03 albertyu03 removed FIXME-Needs Peer CR please get a team member CR then remove this label NEEDS ATTENTION labels Jun 6, 2023
pconrad
pconrad approved these changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@pconrad pconrad left a comment

Choose a reason for hiding this comment

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

LGTM!

@pconrad pconrad merged commit 109ca7b into main Jun 8, 2023
@pconrad pconrad added the 10 10 points label Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeIsNoJoe JoeIsNoJoe JoeIsNoJoe approved these changes

@pconrad pconrad pconrad approved these changes

Assignees
No one assigned
Labels
10 10 points s23-6pm-1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Redesign the user commons page
5 participants
@mfletcher2 @TheRealJBomb @pconrad @JoeIsNoJoe @albertyu03