Skip to content

Conversation

@amjiao
Copy link
Contributor

@amjiao amjiao commented Apr 14, 2025

Overview

Address issue #34

Changes Made

Added EmptyState component, implemented appropriately in PastGamesScreen and HomeScreen

  • UI seemed a bit off from the Figma since the screen layout is slightly different when there aren't any games to display (for example, no Upcoming section when the filteredGames is empty on the HomeScreen)

Test Coverage

Visually correct

@amjiao amjiao marked this pull request as draft April 15, 2025 00:03
zachseidner1 and others added 6 commits April 14, 2025 20:40
* Add missing border

* Bump material version in preparation for pull to refresh

* Fix border on home and past, fix carousel
# Conflicts:
#	app/src/main/java/com/cornellappdev/score/screen/HomeScreen.kt
#	app/src/main/java/com/cornellappdev/score/screen/PastGamesScreen.kt
* Fix sport filter

* Small readability fix
@amjiao amjiao marked this pull request as ready for review April 15, 2025 00:55
@amjiao amjiao requested a review from zachseidner1 April 15, 2025 00:56
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.

Pretty clean for the most part, nice job! Thank you Amy. There's a couple minor things I'd like you to fix.

@amjiao amjiao requested a review from zachseidner1 April 19, 2025 20:54
@zachseidner1 zachseidner1 linked an issue Apr 20, 2025 that may be closed by this pull request
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.

Very nice work! Left a few nits but feel free to just merge if you want lol

Comment on lines 177 to 178
}
if (uiState.filteredGames.isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: better to use else here?

Comment on lines 213 to 217
Column(
modifier = Modifier
.fillMaxSize()
.background(color = Color.White)
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I can remove this column and the preview still works

@amjiao amjiao merged commit 2f9ffe0 into main Apr 22, 2025
1 check passed
@amjiao amjiao deleted the Amy/empty-state branch April 22, 2025 02:31
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.

No Empty States

3 participants