Skip to content

Add network status monitoring#23

Merged
Almond-Peach merged 5 commits intomainfrom
chore/network-status-monitoring
Aug 30, 2023
Merged

Add network status monitoring#23
Almond-Peach merged 5 commits intomainfrom
chore/network-status-monitoring

Conversation

@Almond-Peach
Copy link
Copy Markdown
Contributor

No description provided.

# Conflicts:
#	app/src/main/java/com/simply/birthdayapp/di/DataModule.kt
#	app/src/main/res/values/strings.xml

LaunchedEffect(showNetworkDialog) {
if (keepShowingNetworkDialog && showNetworkDialog.not()) {
lifecycleScope.launch {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you need to open 'scope' here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

lifecycleOwner = this,
)
var keepShowingNetworkDialog by rememberSaveable { mutableStateOf(true) }
var showNetworkDialog by rememberSaveable { mutableStateOf(true) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why are the default values set to 'true'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

By default, a dialog appears when the user is disconnected.

The dialog can be dismissed and shown again after 10 seconds.
The dialog will not be shown again until the app is restarted if the user clicks Don't show.

That is why these default values are set to true.

P.S. Have renamed that variables to align them with the user actions Do not show, Dismiss.

onDismiss: () -> Unit = {},
) {
AlertDialog(onDismissRequest = onDismiss) {
Surface(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you need 'Surface' here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@Almond-Peach Almond-Peach merged commit 91d599d into main Aug 30, 2023
@Almond-Peach Almond-Peach deleted the chore/network-status-monitoring branch August 30, 2023 13:19
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.

2 participants