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

[FR Jetcaster]: Rethink empty, loading, success, and error states in #1376

Open
3 tasks done
arriolac opened this issue Apr 26, 2024 · 3 comments
Open
3 tasks done

[FR Jetcaster]: Rethink empty, loading, success, and error states in #1376

arriolac opened this issue Apr 26, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request triage me Issue that needs to be triaged

Comments

@arriolac
Copy link
Member

arriolac commented Apr 26, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Is this a feature request for one of the samples?

  • Yes, this is a specific request related to this samples repo.

Sample app

Jetcaster

Describe the problem

Jetcaster uses a sealed interface to display different states of the UI. For example:

sealed interface ScreenUiState {
    data object Loading : ScreenUiState

    data class Error(
        val errorMessage: String? = null
    ) : ScreenUiState

    data class Ready(
        /* Params here */
    ) : ScreenUiState
}

However, this may not be the best set up UX-wise as intermediate error states would result in the entire screen showing an error vs. showing an error as part of a partially rendered screen.

Describe the solution

A possible solution would be to fold loading and error states into a single screen state data class.

data class ScreenUiState(
    isLoading: Boolean,
    errorMessage: String?,
    /* Params here */
)

Other proposals are welcome.

Additional context

See discussion in #1363 (comment)

Code of Conduct

  • I agree to follow this project's Code of Conduct
@arriolac arriolac added enhancement New feature or request triage me Issue that needs to be triaged labels Apr 26, 2024
@arriolac arriolac assigned arriolac and unassigned JolandaVerhoef Apr 26, 2024
@yongsuk44
Copy link
Contributor

I personally think that managing multiple distinct states within a single object can make Compose code less readable and harder to manage.

Hmm... If I think about it simply...
I believe that managing each state separately could allow for more versatile responses in various situations.

private val refreshing = MutableStateFlow(false)
private val _state = MutableStateFlow<ScreenUiState>()
private val _error = MutableStateFlow("")

combine (
    // ...
)
    .onStart { refreshing.value = true }
    .onEach { _state.value = it }
    .catch { throwable -> _error.value = throwable.message }
    .collect { refreshing.value = false }

what do you think..?

@arriolac
Copy link
Member Author

I personally think that managing multiple distinct states within a single object can make Compose code less readable and harder to manage.

Yes, that was the impetus for using the sealed interface approach though that did create some limitations as per the description of this issue.

I could imagine a scenario where you might have multiple loading/error states on the same screen. In that case, I would prefer to use a Result in the screen state instead of having a separate flow for loading / errors. For Jetcaster, however, the solution you proposed could work since there aren't any screens where that scenario could apply.

@yongsuk44
Copy link
Contributor

I could imagine a scenario where you might have multiple loading/error states on the same screen. In that case, I would prefer to use a Result in the screen state instead of having a separate flow for loading / errors.

Yes, I also think that using Result or sealed types is more efficient in typical scenarios

After reading the comment by mlykotom, I imagined a scenario where the Success UI is maintained while the Loading UI appears simultaneously. Based on this, I suggested this because I think it might be more efficient to manage the states separately. 🤔

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage me Issue that needs to be triaged
Projects
None yet
Development

No branches or pull requests

3 participants