-
Notifications
You must be signed in to change notification settings - Fork 922
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
Align UI and specs for blockmalware #5380
base: develop
Are you sure you want to change the base?
Conversation
...main/java/com/duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsViewModel.kt
Outdated
Show resolved
Hide resolved
...protection-impl/src/test/java/com/duckduckgo/networkprotection/impl/WgVpnNetworkStackTest.kt
Outdated
Show resolved
Hide resolved
@@ -235,8 +235,9 @@ | |||
<string name="netpDdgDnsByLine">DuckDuckGo routes DNS queries through our DNS servers so your internet provider can\'t see what websites you visit.</string> | |||
|
|||
<!-- Block malware DNS --> | |||
<string name="netpDnsBlockMalwarePrimary">Block Malware</string> | |||
<string name="netpDnsBlockMalwareByline">Block malware with a DNS-level blocklist. If a website doesn\'t load, try turning blocking off.</string> | |||
<string name="netpDnsBlockMalwareHeader">Block lists</string> |
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.
I’ll request translations once we have approved copy (in ship review) since we haven’t had any copy task prior so things could likely change here.
e2e26b4
to
cee77d8
Compare
data class DefaultDns( | ||
val allowChange: Boolean, | ||
val blockMalware: Boolean, | ||
val allowBlockMalware: Boolean, |
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.
I don't think we need this state that carries the kill-switch value.
If we kill-switch the feature we should leave the UX untouched I think, we just stop blocking until we fix whatever broken.
I'd undo the chantes in the activity and view model
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.
I mainly added it as it might be confusing to the users if we don’t update the UI too.
Having that said, I don’t have strong opinions though. It is indeed enough from a kill switch standpoint to just not use the DNS even if the UI says otherwise.
Task/Issue URL: https://app.asana.com/0/1203137811378537/1208939629128396/f
Description
This PR aligns the UI for VPN blockmalware feature to the figma designs. We are also making the feature available to the users.
Steps to test this PR
https://app.asana.com/0/1203137811378537/1208939629128403/f