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

Switch to compose state and kotlin flows - 2. iteration #287

Merged

Conversation

sunkup
Copy link
Member

@sunkup sunkup commented Apr 17, 2024

Purpose

Replacing LiveData with compose state and flows, will in the future hopefully prevent the issues that came with LiveData. Also it makes for easier integration with DAVx5.

Short description

This is the 2. Iteration of the switch from LiveData to compose state and kotlin flows. See the 1. iteration here.

The 1. Iteration took care of the Database Access Objects. It deprecated LiveData returning methods and added their respective Flow returning variants.

In this 2. iteration models and activities are rewritten, by replacing all LiveData usages in both models and activities, excluding any 1. iteration DAO changes, since this PR would grow too big otherwise.

The 3. iteration will take care of using the new Flow returning methods in DAOs and remove the last remaining LiveData usages.

This PR:

  • Prefers compose state over StateFlows where possible
  • Uses StateFlow in CredentialsModel and SubscriptionSettingsModel since their flow type members are merged in EditCalendarActivity and it is not possible to merge compose states, to my knowledge. In any case, the declaration of modelsDirty needs special attention.
  • introduces uiState data classes with private set where possible

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup linked an issue Apr 17, 2024 that may be closed by this pull request
9 tasks
@sunkup sunkup force-pushed the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch from fe4805f to 97be8fb Compare April 17, 2024 10:00
@sunkup sunkup changed the title Switch to compose state and kotlin flows - 2. iteration Switch to compose state and kotlin flows Apr 18, 2024
@sunkup sunkup force-pushed the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch from 97be8fb to a34362e Compare April 18, 2024 07:46
@sunkup sunkup changed the title Switch to compose state and kotlin flows Switch to compose state and kotlin flows - 2. iteration Apr 18, 2024
@sunkup sunkup force-pushed the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch 2 times, most recently from 201a50a to b252c4c Compare April 18, 2024 10:20
@sunkup sunkup marked this pull request as ready for review April 18, 2024 12:07
@sunkup sunkup requested a review from ArnyminerZ April 18, 2024 12:07
@sunkup sunkup added this to the 2.2.2 milestone Apr 18, 2024
@sunkup sunkup force-pushed the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch from 9bc712e to e399a57 Compare April 18, 2024 12:17
Copy link
Member

@ArnyminerZ ArnyminerZ left a comment

Choose a reason for hiding this comment

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

I have not commented on everything, but as stated, use Flow or Compose states as:

var variable by mutableStateOf(...)
  private set

And then in the composable, just use =:

val variable = model.variable

And set all values from the viewmodels through setters as needed.

@sunkup sunkup force-pushed the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch from e399a57 to 114956d Compare April 29, 2024 12:06
@sunkup
Copy link
Member Author

sunkup commented Apr 30, 2024

I think this should be it for now? Should turn a lot prettier with the 3rd and 4th iterations.

@sunkup sunkup requested a review from ArnyminerZ April 30, 2024 08:48
Copy link
Member

@ArnyminerZ ArnyminerZ left a comment

Choose a reason for hiding this comment

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

Just one state missing 😉 (see unresolved conversation)

@sunkup sunkup requested a review from ArnyminerZ April 30, 2024 09:28
@ArnyminerZ ArnyminerZ merged commit d464027 into dev Apr 30, 2024
7 checks passed
@ArnyminerZ ArnyminerZ deleted the 278-switch-to-compose-state-and-kotlin-flows-2-iteration branch April 30, 2024 09:33
@sunkup sunkup added the refactoring Quality improvement of existing functions label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Quality improvement of existing functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to compose state and kotlin flows - 2. iteration
2 participants