-
Notifications
You must be signed in to change notification settings - Fork 872
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
Pre release autofill #1274
Pre release autofill #1274
Conversation
…feature/marcos/pre-release-autofill � Conflicts: � app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt � app/src/main/java/com/duckduckgo/app/email/db/EmailDataStore.kt � app/src/main/java/com/duckduckgo/app/email/di/EmailModule.kt � app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt � app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt
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.
Code review is done, GJ @marcosholgado, everything was really well structured.
I'm going to start testing this now, so I will approve the PR once I ensure everything works as expected. However, sending to you some minor comments. Thanks.
app/src/main/java/com/duckduckgo/app/beta/BetaFeaturesActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/email/ui/EmailProtectionActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/email/ui/EmailProtectionViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/email/ui/EmailProtectionViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/notification/NotificationSender.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/email/waitlist/WaitlistWorkRequestBuilder.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/duckduckgo/app/email/ui/EmailProtectionSignOutViewModelTest.kt
Show resolved
Hide resolved
…feature/marcos/pre-release-autofill � Conflicts: � app/src/main/res/values/themes.xml � common-ui/src/main/res/values/attrs.xml
…feature/marcos/pre-release-autofill � Conflicts: � app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt � app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt
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.
Code review finished, sending some minor comments. Feel free to address them if you agree with them.
I will approve the PR once I'm done with the manual testing. But I wanted to give send you the review in advance.
app/src/main/java/com/duckduckgo/app/email/ui/EmailProtectionSignOutViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/email/ui/EmailProtectionFragment.kt
Show resolved
Hide resolved
setClickableSpan(binding.emailPrivacyDescription, R.string.emailProtectionDescriptionJoin, listOf(readBlogSpan)) | ||
} | ||
|
||
private val getNotificationSpan = object : NonUnderlinedClickableSpan() { |
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.
should all these val
be at the top of the file?
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.
👍
~ limitations under the License. | ||
--> | ||
|
||
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" |
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.
SettingsPillWithSubtitle
is already a LinearLayout
, you can use merge
and remove another nested linearlayout here.
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.
Testing done! 👍
Let's gooooo
…feature/marcos/pre-release-autofill
# Conflicts: # app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt # app/src/main/java/com/duckduckgo/app/settings/SettingsActivity.kt # app/src/main/java/com/duckduckgo/app/settings/SettingsViewModel.kt
Task/Issue URL: https://app.asana.com/0/inbox/1125189844075764/1200425975637284/1200442170423677
Tech Design URL:
CC:
Description:
This PR adds the pre-release features for autofill
Steps to test this PR:
Test cases are in the Asana task
Internal references:
Software Engineering Expectations
Technical Design Template