fix blink favorite icon

[kyanro]

Issue

• close When returning after performing a favorite operation on SearchScreen, the favorite icon blinks momentarily #609

Overview (Required)

The probable causes are:

The value of collectAsRetainedState is not collected while navigating to another screen.
When returning to the original screen, the value is collected
but the update of the value likely occurs based on the execution timing of LaunchedEffect, so until the value is updated, the displayed value is not the latest.

• Change getFavoriteSessionStream return type to StateFlow
  • To allow setting the initial value synchronously
• Change safeCollectAsRetainedState to collectAsState
  • To collect the latest value when returning to the previous screen

Movie (Optional)

Before	After
before_Screen_recording_20240904_002135.mp4	after_Screen_recording_20240906_002004.mp4

[takahirom]

Thank you for your insights on the code. I think this PR introduces several changes. What do you think is the cause? If we use memory cache, is rememberRetained fine? It is a little difficult to manage if we add a source of information.

[kyanro]

Thank you for your comment.

What do you think is the cause?

I did some further investigation.

The probable causes are:

• The value of collectAsRetainedState is not collected while navigating to another screen.
• When returning to the original screen, the value is collected
  • but the update of the value likely occurs based on the execution timing of LaunchedEffect, so until the value is updated, the displayed value is not the latest.

If we use memory cache, is rememberRetained fine?

The problem seems to be that the value during the transition to another screen cannot be reflected, so it doesn’t seem that rememberRetained can solve the issue.

It is a little difficult to manage if we add a source of information.

I agree with that.

How about changing getFavoriteSessionStream to return a StateFlow instead of exposing the memory cache, and using collectAsState so that the latest value is always reflected when returning to the original screen?

I believe there may be some issues with this approach as well, so once I push the code, I’d appreciate it if you could review it.

[kyanro]

Updated the source code and the overview 😄

[github-actions]

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

[takahirom]

This could be an unmanaged dispatcher in tests, which could result in flaky tests. I think we need to pass a dispatcher for this.

[kyanro]

Thank you for your comment.

I modified it so that testDispatcher can be used during testing.
Does this fix seem okay to you?

move coroutineScope field to constructor parameter
Add TestUserDataStoreModule

[github-actions]

Snapshot diff report

File name	Image
FavoritesScreenTest[ FavoritesScreen - wh en server is operati onal - setup many fa vorite sessions - cl ick first session bo okmark - it should d isplay favorite sess ion without first se ssion]_compare.png
FavoritesScreenTest[ FavoritesScreen - wh en device is tablet - it should show tim etable items]_compar e.png
FavoritesScreenTest[ FavoritesScreen - wh en server is operati onal - setup many fa vorite sessions - sc roll favorites - it should first session is not displayed]_c ompare.png

[takahirom]

I'm not sure, but the tests don't show all the favorite sessions. 👀
I'm also uncertain if we should have a CoroutineScope in DataStore, as it might consume resources even when the app is in the background.

[kyanro]

Thank you.
I will investigate the issue regarding all favorite sessions not being displayed.

I also share your concern about having a CoroutineScope in DataStore, so I will try exploring alternative approaches, though it may be quite challenging.

If I can’t resolve it, I may give up and close this pull request, so would it be possible to assign someone else?

[kyanro]

I apologize. I tried various things, but I couldn't resolve the issue successfully.

I will continue to investigate, but I would like to unassign myself and close this pull request for now.