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
Update copy when deleting passwords to consider whether sync enabled or not #4445
base: develop
Are you sure you want to change the base?
Update copy when deleting passwords to consider whether sync enabled or not #4445
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @CDRussell and the rest of your teammates on Graphite |
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.
lgtm, but leaving some comments about naming and asking to move the sync method call to background. I will go through the test cases when the background part is done so it's final. But those are the only 2 things I've seen.
...ckduckgo/autofill/impl/ui/credential/management/viewing/AutofillManagementCredentialsMode.kt
Outdated
Show resolved
Hide resolved
.../com/duckduckgo/autofill/impl/ui/credential/management/viewing/AutofillManagementListMode.kt
Outdated
Show resolved
Hide resolved
...duckduckgo/autofill/impl/ui/credential/management/viewing/AutofillManagementStringBuilder.kt
Outdated
Show resolved
Hide resolved
...duckduckgo/autofill/impl/ui/credential/management/viewing/AutofillManagementStringBuilder.kt
Outdated
Show resolved
Hide resolved
c06f265
to
e4d11bf
Compare
@cmonfortep moved sync check to background thread and renamed string builder functions. ready for review. (can ignore the lint error since that'll be solved in the translations branch for this) |
8b44e53
to
d7ecadb
Compare
d7ecadb
to
3510012
Compare
@cmonfortep pinging for re-review Note, I had to change how we differentiate between singular and plural cases. Before, got caught out with the "implied plural" lint warnings. So now we check in code if number of elements == 1 and use a normal string if so, and else grab the plural form. |
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.
LGTM
Task/Issue URL: https://app.asana.com/0/488551667048375/1206638124888258/f
Description
Copy changes around deleting passwords (now showing a different message when sync is enabled vs disabled).
Translations will come in #4446
Steps to test this PR
Sync Disabled
single saved password
multiple saved passwords
Sync Enabled
multiple saved passwords
single saved password