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

Editing profile: No change warning on leave. #3972

Merged

Conversation

Gryzor
Copy link
Collaborator

@Gryzor Gryzor commented Aug 19, 2023

Objective

  • Prevent data loss when the user inadvertently hits back or wants to leave the profile edition with unsaved changes.

Description

  • To limit the number of changes to the existing codebase, I merely re-used the same method used by save() in the ViewModel to decide whether to make a network request or simply return the profile as-is.
  • A bit of code juggling around in the ViewModel and I was able to use the logic for all the encoding of each profile field (Which is what the ViewModel caches in memory). Thanks @Lakoja for improving this in the VM.
  • A couple of internal data classes used as helpers to move all the fields around (now that they are no longer used in one single place) were introduced.

Potential Optimizations

  • The profile encoding is done twice (once for checking, and then again if the user has to actually save it). I'd say this is a negligible price to pay, since the alternative would be to create a different set of comparisons and/or keeping another profile in memory for the purpose of comparison.

Visual Improvement

  • I believe the Dialog is difficult to see, but it's being displayed with Tusky's theme. Perhaps there's a better style to apply in this case? (or maybe the edit profile activity shouldn't have the same background color as dialogs?!)

Issue

@Gryzor Gryzor requested a review from nikclayton August 19, 2023 15:52
@Gryzor Gryzor self-assigned this Aug 19, 2023
@Gryzor Gryzor changed the title Editing profile: No change warning on leave #3486 Editing profile: No change warning on leave. Aug 19, 2023
Martin Marconcini and others added 3 commits August 24, 2023 10:06
* Remove dialog title and string
* Rename Data Class
* Use liveData.value directly when possible
@Gryzor Gryzor removed the request for review from nikclayton August 27, 2023 23:26
@Tak Tak merged commit 10a6b16 into tuskyapp:develop Aug 28, 2023
4 checks passed
@Gryzor Gryzor deleted the prompt_to_save_before_leaving_changed_profile branch August 28, 2023 12:17
@charlag charlag modified the milestone: Tusky 24 Nov 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants