Skip to content

Conversation

@EmilJiang
Copy link
Contributor

@EmilJiang EmilJiang commented Apr 21, 2025

I added navigation in the game details screen. I am not sure if this is the best way to do it, but it was the only way I could think of after trying many things. To test, the best way I found was to go into past scores and go from there as many of the future games do not have scoring summaries. #46

@EmilJiang EmilJiang requested a review from zachseidner1 April 21, 2025 03:20
Copy link
Collaborator

@zachseidner1 zachseidner1 left a comment

Choose a reason for hiding this comment

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

After a bit of research I found out that this should definitely be possible to do this with type safety. See this Stack Overflow, we need to use the typeMap parameter of composable in our root navigator. If you have time today, I'd love for you to take a crack at it. Otherwise we can leave it like this and create a GitHub issue, because it's not that bad.

Regardless of what you decide to do, you need to fix the icon for navigating to the scoring summary.

Comment on lines +183 to +189
Icon(
painter = painterResource(id = R.drawable.ic_right_chevron),
contentDescription = "Back button",
modifier = Modifier
.width(24.dp)
.height(24.dp),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

image the icon appears really off here?

@EmilJiang
Copy link
Contributor Author

I saw that stackoverflow, but I did not know how to add the typeMap as that requires some sort of external library, and I think I need to create a ListNavType which I do not really know how to do.
The arrow is fixed.
Screenshot 2025-04-21 at 9 19 17 PM
By disable in #60, do you just mean remove? If so, my new commit address it.
Fixed the ScoreBox #61. This has not been thoroughly tested as the backend is really slow for me.
Image is now correct with the team for Scoring Summary, and ScoringSummaryScreen. Checked the real players name to make sure it matched the team.
Screenshot 2025-04-21 at 9 56 52 PM
Screenshot 2025-04-21 at 9 57 47 PM

Copy link
Collaborator

@zachseidner1 zachseidner1 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@zachseidner1 zachseidner1 merged commit ecf9a70 into main Apr 22, 2025
1 check passed
@zachseidner1 zachseidner1 deleted the emil_jiang_navigate branch April 22, 2025 15:01
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.

Scoring Box takes a minimum of 4 scores Disable Add to Calendar Navigate to scoring summary page

3 participants