-
Notifications
You must be signed in to change notification settings - Fork 360
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
Fix DnsDialog shortly showing error after adding new DNS entry #6309
Fix DnsDialog shortly showing error after adding new DNS entry #6309
Conversation
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.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Rawa)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt
line 33 at r2 (raw file):
data class DnsDialogViewState( val ipAddress: String, val validationResult: ValidationError?,
Probaby clearer if this is called validationError?
android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt
line 59 at r2 (raw file):
val uiState: StateFlow<DnsDialogViewState> = combine(_ipAddressInput, repository.settingsUpdates.filterNotNull().take(1)) {
I wonder if this should be outside of this view model since it is probably something that we will use somewhere else? Like a latestSettings
function somewhere?
android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt
line 87 at r2 (raw file):
} private fun String.validateDnsEntry(
Looks really nice 👍
276082b
to
f2cd00c
Compare
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.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @Pururun)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt
line 33 at r2 (raw file):
Previously, Pururun (Jonatan Rhodin) wrote…
Probaby clearer if this is called validationError?
Yes, fixed!
android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt
line 59 at r2 (raw file):
Previously, Pururun (Jonatan Rhodin) wrote…
I wonder if this should be outside of this view model since it is probably something that we will use somewhere else? Like a
latestSettings
function somewhere?
Fixed, this by changing the logic of addCustomDns
. Now we get back the index, and then update that when it was returned.
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.
Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
f2cd00c
to
125fc8c
Compare
Blocked by other PR, up for opinions about how we solve the prevention of the Error appearing on re-rendering with new data.
This change is