-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Reader Customization] Allow dragging to close Preferences bottom sheet #20640
Changes from all commits
3a4e770
2ddd072
dc2d978
6766f88
18a893e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,12 @@ class ReaderReadingPreferencesViewModel @Inject constructor( | |
} | ||
|
||
fun onScreenClosed() { | ||
if (isDirty()) { | ||
// here we assume the code for saving preferences has been called before reaching this point | ||
launch { | ||
_actionEvents.emit(ActionEvent.UpdatePostDetails) | ||
} | ||
} | ||
readingPreferencesTracker.trackScreenClosed() | ||
} | ||
|
||
|
@@ -65,15 +71,24 @@ class ReaderReadingPreferencesViewModel @Inject constructor( | |
readingPreferencesTracker.trackItemTapped(fontSize) | ||
} | ||
|
||
fun saveReadingPreferencesAndClose() { | ||
/** | ||
* An exit action has been triggered by the user. This means that we need to save the current preferences and emit | ||
* the close event, so the dialog is dismissed. | ||
*/ | ||
fun onExitActionClick() { | ||
launch { | ||
val currentPreferences = currentReadingPreferences.value | ||
val isDirty = currentPreferences != originalReadingPreferences | ||
if (isDirty) { | ||
saveReadingPreferences(currentPreferences) | ||
readingPreferencesTracker.trackSaved(currentPreferences) | ||
} | ||
_actionEvents.emit(ActionEvent.Close(isDirty)) | ||
saveReadingPreferencesInternal() | ||
_actionEvents.emit(ActionEvent.Close) | ||
} | ||
} | ||
|
||
/** | ||
* The bottom sheet has been hidden by the user, which means the dismiss process is already on its way. All we need | ||
* to do is save the current preferences. | ||
*/ | ||
fun onBottomSheetHidden() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can remove the function curly braces There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a big fan of that approach since doing that causes the function to return a I get that using |
||
launch { | ||
saveReadingPreferencesInternal() | ||
} | ||
} | ||
|
||
|
@@ -84,8 +99,19 @@ class ReaderReadingPreferencesViewModel @Inject constructor( | |
} | ||
} | ||
|
||
private suspend fun saveReadingPreferencesInternal() { | ||
val currentPreferences = currentReadingPreferences.value | ||
if (isDirty()) { | ||
saveReadingPreferences(currentPreferences) | ||
readingPreferencesTracker.trackSaved(currentPreferences) | ||
} | ||
} | ||
|
||
private fun isDirty(): Boolean = currentReadingPreferences.value != originalReadingPreferences | ||
|
||
sealed interface ActionEvent { | ||
data class Close(val isDirty: Boolean) : ActionEvent | ||
data object Close : ActionEvent | ||
data object UpdatePostDetails : ActionEvent | ||
data class UpdateStatusBarColor(val theme: ReaderReadingPreferences.Theme) : ActionEvent | ||
data class OpenWebView(val url: String) : ActionEvent | ||
} | ||
|
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.
Ditto here. Just to save some curly braces.
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.
Same reasoning for not returning
Job
here.