Fireproof websites empty state and Feature pixels #810

cmonfortep · 2020-05-04T14:11:04Z

Task/Issue URL: https://app.asana.com/0/0/1170962402828809/f
Tech Design URL:
CC:

Description:

This PR includes two different things:

Empty state for fireproof websites screen
Feature pixels

Implement empty state for fireproof websites screen

Light	Dark

Implement feature pixels

User clicks on "fireproof a website"
User undo "fireproof website" action (confirmation snackbar after fireproffing a website)
User removed a website from "fireproof websites"

Steps to test this PR:

Test 1: fireproof websites empty state

Fresh install
Access settings -> fireproof websites
Ensure list is empty and "No websites fireproofed yet"
Go back to browser
Fireproof any website
Navigate to settings -> fireproof websites again
Ensure list is not empty
Remove all websites from the list
Ensure "No websites fireproofed yet" appeared again

Test 2: fireproof website pixel

Go back to browser
Fireproof any website
Ensure pixel sent: m_fw_a

Test 3: Undo fireproof website pixel

Go back to browser
Fireproof any website
Click on Undo snackbar action
Ensure pixel sent: m_fw_u

Test 4: Remove fireproof website pixel

Go back to browser
Fireproof any website
Navigate to settings -> fireproof websites again
Remove a website from the list
Ensure pixel sent: m_fw_d

Test 5: Description on top

Ensure description always appears on top of empty state or list of fireproof websites

Internal references:

Software Engineering Expectations
Technical Design Template

…eserved

* Introduced subtle background for favicon * changes to align with current design

disabled if home screen or domain exists in database enabled if doesn't exist in database

…creen

…oofwebsites

…ite in bold

malmstein

The pixels are sent and functionality works as expected, good job!

I've added a few comments for you regarding code consistency. Regarding the UI, looks like the text size of "No websites fireproofed yet" does not match the designs. Can you double check with the design team?

app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsiteAdapter.kt

malmstein · 2020-05-05T15:43:45Z

app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesActivity.kt

-        adapter = FireproofWebsiteAdapter(viewModel, R.string.fireproofWebsiteFeatureDescription)
+        adapter = FireproofWebsiteAdapter(
+            viewModel = viewModel,
+            emptyListHintStringRes = R.string.fireproofWebsiteEmptyListHint,


see comment about fireproofWebsiteEmptyHint.setText(text)

app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesViewModel.kt

app/src/main/res/layout/view_fireproof_website_empty_hint.xml

cmonfortep · 2020-05-06T06:05:25Z

@malmstein both hints (I didn't change the one from bookmarks) have 16sp, and they share the same style.
The design link to Figma is available inside the asana task. Here's a screenshot from there where you can see the 16sp reference.

I understand the difference is just a matter of using different screen size/density.

Bookmarks	Fireproof

malmstein · 2020-05-06T07:02:46Z

@cmonfortep I understand what you mean about using the dimensions from Figma. This has been a problem before but we don't need to fix this at this stage.

I'd recommend you do a Design Review before moving to Product Review. You can take https://app.asana.com/0/1157893581871903/1170225026602691 as an example, it really helped ironing out the final details and ensure everyone was happy with the end result

cmonfortep · 2020-05-06T07:17:37Z

@malmstein maybe we can talk offline about problems with dimensions from Figma, I'm not aware of any problem and I would like to know more.

About design review: I already went through it -> https://app.asana.com/0/1129367408583813/1172778240040897/f

malmstein · 2020-05-06T08:32:00Z

@cmonfortep I meant a review after you've implemented the changes and built a version that the designer can play with. This avoids comments like "I might be put off a little bit by the resolution from your phone." and reduces the friction that will come out of Product Review when other stakeholders actually install the app.

malmstein

We walked through the changes and we are both happy with the result. Good job @cmonfortep!

cmonfortep · 2020-05-08T08:10:03Z

@malmstein during product review we decided to change the description to the top of the screen. Can you review the last commit?
product review -> https://app.asana.com/0/1142021229838617/1174277015894962/f

malmstein · 2020-05-08T09:30:48Z

@cmonfortep after the last update the fireproof list no longer displays properly. Adding multiple sites will show multiple headers instead of the actual site

cmonfortep · 2020-05-08T09:32:23Z

Thanks @malmstein! I will check that ASAP 😵

cmonfortep · 2020-05-08T09:47:38Z

@malmstein fixed. I've just tested it with empty/single/multiple sites, can you review it again?

malmstein

Tested with the latest changes and works. Good job!

…ian/ui_improvements_feature_pixels # Conflicts: # app/src/androidTest/java/com/duckduckgo/app/fire/SQLCookieRemoverTest.kt # app/src/androidTest/java/com/duckduckgo/app/fire/WebViewCookieManagerTest.kt # app/src/androidTest/java/com/duckduckgo/app/fire/WebViewDatabaseLocatorTest.kt # app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt # app/src/main/java/com/duckduckgo/app/fire/CookieRemover.kt # app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt

* Fireproof websites UI (#796) * Create new database table to persist sites where cookies should be preserved * Removed divider from bookmarks list * bookmarks title show in single line * background favicon compatible with dark theme * introduce fireproof site option menu * Fireproof option menu reacts to database state. * Fireproof website screen created * Remove cookies preserving fireproof websites (#808) * Implement logic to remove cookies preserving the ones related to fireproof website. - Try to directly remove cookies from WebView database, preserving the ones with hosts related to a fireproof website - If process fails, fallback to remove all the cookies to avoid any leak - Send pixels in the following scenarios: - database path not found - database can't be opened - delete query fails - database corruption * Fireproof websites empty state and Feature pixels (#810) * Empty state for fireproof websites screen * Feature pixels - User clicks on "fireproof a website" - User undo "fireproof website" action (confirmation snackbar after fireproffing a website) - User removed a website from "fireproof websites"

cmonfortep added 30 commits April 17, 2020 10:15

(wip) removing cookies manually based on bookmarks

a6a45f3

(wip) tidy up and fallback to previous logic when manual process fails

bdfbdd3

(wip) prototype - send pixels for errors and remove cookie duration

4a4ed29

Merge branch 'develop' into feature/cristian/webview_database_datasource

28530da

prototype version

4719e7f

Create new database table to persist sites where cookies should be pr…

fb18f01

…eserved

* Removed divider from bookmarks list

2110a51

* Introduced subtle background for favicon * changes to align with current design

bookmarks title show in single line

2ee0153

background favicon compatible with dark theme

0111b3d

Include fireproof websites entry cell in settings

f4825a3

introduce methods to insert or remove preserver cookies entities

3b463a3

inject dao into viewmodel

e5cb752

add untranslated strings

61e15b1

introduce fireproof site option menu and logic for actions

11f27d6

Fireproof option menu reacts to database state.

2cabcd4

disabled if home screen or domain exists in database enabled if doesn't exist in database

persist original url in order to show favicon in fireproof websites s…

018dedc

…creen

extract original url when fireproof website clicked

9e2fbfb

Fireproof website screen created + logic

829f112

observe fireproof websites dao

d0a0863

Big naming refactor. Keeping consistent the domain language to firepr…

1bd99ee

…oofwebsites

Revert non related work committed

d92fc84

Merge branch 'develop' into feature/cristian/preserve_sites_ui

9db51ef

adding more copies to untranslated strings

2a09e31

Showing single action on Fireproof websites screen item overflow menu

7a508d2

showing snackbar with fireproof website in bold

7b15941

delete confirmation dialog when removing fireproof website shows webs…

1eff472

…ite in bold

clean up

b6dac2d

renaming on activity layout for fireproof website screen

c628796

apply code style

25e7d2e

change from coroutine to sync code using local list

5e81a05

cmonfortep requested review from marcosholgado and subsymbolic as code owners May 5, 2020 07:35

cmonfortep assigned malmstein May 5, 2020

malmstein requested changes May 5, 2020

View reviewed changes

cmonfortep added 2 commits May 6, 2020 08:54

send pixel only when website added + test

44e7978

add test case for sending pixel snackbar undo action

2dd91e1

cmonfortep added 2 commits May 6, 2020 09:03

user attr related with this feature instead of bookmark reference

fac4041

moving pixel execution inside coroutine block

b7630f0

cmonfortep added 2 commits May 6, 2020 09:22

moving stringRes into xml item layout and remove bind logic from adapter

6cc9144

ui tweak to make text spread inside the textview.

08bdca2

cmonfortep requested a review from malmstein May 6, 2020 07:35

malmstein approved these changes May 6, 2020

View reviewed changes

move description text to the top of the list

6a8df2e

malmstein self-requested a review May 8, 2020 09:28

fix: multiple descriptions being shown

e564cc2

malmstein approved these changes May 8, 2020

View reviewed changes

cmonfortep changed the base branch from feature/cristian/webview_database_datasource to feature/cristian/fireproof_websites May 13, 2020 12:48

cmonfortep merged commit 48df41f into feature/cristian/fireproof_websites May 13, 2020

cmonfortep deleted the feature/cristian/ui_improvements_feature_pixels branch May 13, 2020 13:21

cmonfortep mentioned this pull request May 13, 2020

Android: Site-level "fireproof" feature to preserve logins #829

Merged

Fireproof websites empty state and Feature pixels #810

Fireproof websites empty state and Feature pixels #810

Uh oh!

Conversation

cmonfortep commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Steps to test this PR:

Internal references:

Uh oh!

malmstein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

malmstein May 5, 2020

Choose a reason for hiding this comment

Uh oh!

cmonfortep May 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmonfortep commented May 6, 2020

Uh oh!

malmstein commented May 6, 2020

Uh oh!

cmonfortep commented May 6, 2020

Uh oh!

malmstein commented May 6, 2020

Uh oh!

malmstein left a comment

Choose a reason for hiding this comment

Uh oh!

cmonfortep commented May 8, 2020

Uh oh!

malmstein commented May 8, 2020

Uh oh!

cmonfortep commented May 8, 2020

Uh oh!

cmonfortep commented May 8, 2020

Uh oh!

malmstein left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cmonfortep commented May 4, 2020 •

edited

Loading