♻️ [SponsorsScreen]Added loading indicator since it is missing. #950
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
takahirom
reviewed
Sep 5, 2024
| val supporters = sponsors.filter { it.plan == SUPPORTER }.toPersistentList() | ||
|
|
||
| val platinumSponsorsUiState = if (platinumSponsors.isNotEmpty()) { | ||
| PlatinumSponsorsUiState.Exists( |
There was a problem hiding this comment.
How about making one big state instead of making for each category?
sealed interface SponsorsListUiState{
data class Exists(
val platinumSponsors: PersistentList<Sponsor>,
val goldSponsors: PersistentList<Sponsor>,
val supporters: PersistentList<Sponsor>,
)
object Loading
}There was a problem hiding this comment.
@takahirom
The issues you pointed out have already been dealt with, so your PR is ready to be reviewed. 🫡
d80ba13
…at they are all held as a single UiState.
takahirom
reviewed
Sep 6, 2024
| sponsor = sponsor, | ||
| onSponsorsItemClick = onSponsorsItemClick, | ||
| ) | ||
| when (uiState) { |
There was a problem hiding this comment.
Sorry, but if you want to show the progress for each section, the previous UI state definition is better. (I have so many PRs to review, and I don't have time to look into this a lot 🙇). Can we have a Slot API pattern-like method for each category? These boilerplate codes are a little messy.
There was a problem hiding this comment.
@takahirom
I restored UiState and reduced redundant processing. 🫡
…changed so that they are all held as a single UiState." This reverts commit d80ba13.
github-actions
bot
had a problem deploying
to
deploygate-distribution
|
Snapshot diff report
|
![SponsorsScreenTest[SponsorsScreen - when server is down - when launch - it should does not show sponsors and show snackbar]_compare.png](https://github.com/DroidKaigi/conference-app-2024/blob/companion_enhancement/implement_loading_ui_state_sponsor_screen/./screenshot-diff-reports/feature/sponsors/build/outputs/roborazzi/SponsorsScreenTest%5BSponsorsScreen%20-%20when%20server%20is%20down%20-%20when%20launch%20-%20it%20should%20does%20not%20show%20sponsors%20and%20show%20snackbar%5D_compare.png){kind=link}
takahirom
approved these changes
Sep 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Overview (Required)
Movie (Optional)
before.mp4
after.mp4