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.

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

♻️ [iOS & Android ContributorsScreen]Added loading indicator since it is missing. #948

Conversation

Corvus400
Copy link
Contributor

@Corvus400 Corvus400 commented Sep 3, 2024

Issue

  • None.

Overview (Required)

  • I modified the UIState so that it can display a loading screen.

Movie (Optional)

Before After
before.android.mp4
after.android.mp4

Copy link

github-actions bot commented Sep 3, 2024

Detekt check failed. Please run ./gradlew detekt --auto-correct to fix the issues.

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 18:14 Inactive
@github-actions github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 18:24 Inactive
Contributors Since it seems that Loading and Exists are not output to XCFrameWork when UiState is a sealed interface, I modified it like this.
@github-actions github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 21:06 Inactive
@Corvus400 Corvus400 changed the title ♻️ [ContributorsScreen]Added loading indicator since it is missing. ♻️ [iOS & Android ContributorsScreen]Added loading indicator since it is missing. Sep 3, 2024
@Corvus400 Corvus400 marked this pull request as ready for review September 3, 2024 22:10
Comment on lines +52 to +63
sealed class ContributorsUiState {
abstract val userMessageStateHolder: UserMessageStateHolder
}

class Loading(
override val userMessageStateHolder: UserMessageStateHolder,
) : ContributorsUiState()

class Exists(
override val userMessageStateHolder: UserMessageStateHolder,
val contributors: PersistentList<Contributor>,
val userMessageStateHolder: UserMessageStateHolder,
)
) : ContributorsUiState()
Copy link
Contributor Author

@Corvus400 Corvus400 Sep 3, 2024

Choose a reason for hiding this comment

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

At first, UiState was configured with sealed interface, but since it is not included in XCFramework and cannot be referenced on the iOS side, it has been made into a sealed class and then into this form.
734b8b3#diff-dd1ff2a856844eaf95d9daf6c02a417e43b971fb53282a4d81e398b0a89ca639L52-L63

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 3, 2024 22:14 Inactive
@Corvus400
Copy link
Contributor Author

The indicator display on the iOS side cannot be confirmed because there is a problem where the contributor screen crashes when there is no network connection on the iOS side. 🙏
#954

Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

For Android side, looks great

Copy link
Member

@ry-itto ry-itto left a comment

Choose a reason for hiding this comment

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

Looks good about iOS side 👍🏼

@ry-itto ry-itto merged commit b1daaea into DroidKaigi:main Sep 5, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants