-
Notifications
You must be signed in to change notification settings - Fork 0
Omj 8/task3 #23
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
base: Omj-8/main
Are you sure you want to change the base?
Omj 8/task3 #23
Conversation
| SwipeRefresh( | ||
| state = swipeRefreshState, | ||
| onRefresh = { viewModel.refreshMessages() }, | ||
| modifier = Modifier.fillMaxSize() | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[必須]
AccompanistのSwipeRefreshは最新のAccompanistでは削除されました。
google/accompanist#1797
AccompanistからSwipeRefreshが消えた理由はCompose公式でPullToRefreshがサポートされたからです。
なので、公式のPullToRefreshBoxを使う形に変えたほうが良さそうです。
https://developer.android.com/develop/ui/compose/components/pull-to-refresh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ビルド時にワーニングも出てました
w: file:///Users/i001197/work/android/android-developer-journey/feature/communicate/src/main/java/com/cybozu/sample/kintone/spaces/feature/communicate/thread/ThreadScreen.kt:68:29 'fun rememberSwipeRefreshState(isRefreshing: Boolean): SwipeRefreshState' is deprecated. accompanist/swiperefresh is deprecated.
The androidx.compose equivalent of rememberSwipeRefreshState() is rememberPullRefreshState().
For more migration information, please visit https://google.github.io/accompanist/swiperefresh/#migration.
| Column(horizontalAlignment = Alignment.CenterHorizontally) { | ||
| CircularProgressIndicator() | ||
| Spacer(modifier = Modifier.height(16.dp)) | ||
| Text("Loarding...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Good]
ロード中、インジケータだけじゃなくてテキストでロード状態を伝えるテキストを出すのはユーザに優しくていいですね!
(たまたま誤字見つけました)
| Text("Loarding...") | |
| Text("Loading...") |
| is ThreadUiState.Initial -> { | ||
| Text("Initial State") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[必須]
ユーザに「Initial State」と提示するのは伝わらなさそうに思いました。
InitialのときのTextは無くて良さそうですね。
| successState.threadMessage[1].body shouldBe "thread-2" | ||
| successState.threadMessage[1].creator shouldBe Creator(name = "name2") | ||
|
|
||
| cancelAndIgnoreRemainingEvents() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[質問]
これってなんで必要なんです?
ViewModelの実装をみていると、2回目のSuccessの後はState更新がないのが期待動作のように見えるんですが。。。
| val initialState = awaitItem() | ||
| (initialState is ThreadUiState.Initial) shouldBe true | ||
|
|
||
| val loadingState = awaitItem() | ||
| (loadingState is ThreadUiState.Loading) shouldBe true | ||
|
|
||
| val loadedState = awaitItem() | ||
| (loadedState is ThreadUiState.Success) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[任意]
余裕があれば、1回目のデータと2回目のデータが違うことも検証してみてください!
例(このパターンじゃなくてもOKです)
1回目は2つのメッセージが取れる
2回目は3つのメッセージが取れる
task3