Skip to content

Conversation

@cmonfortep
Copy link
Contributor

@cmonfortep cmonfortep commented May 13, 2021

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

Description:
This PR introduces favorites in the Android app.
List of new features:

  • Bookmark screen redesigned to show favorites
  • Users can add favorites from the overflow menu
  • Favicon logic improved to fetch high-res favicons
  • Quick access grid on the home tab (and on the SystemSearchActivity)
  • Quick access grid allow users to drag-drop and rearrange favorites (also edit and delete via long-press)
  • autocomplete shows favorites as results
  • Focusing the URL bar when browsing also show a grid with favorites

Steps to test this PR:

Fresh install

  1. Fresh install
  2. Visit any website and add it as favorite
  3. During onboarding, favorites will not appear in the home tab if any Dax cta is active
  4. Finish onboarding
  5. Ensure favorites are shown in the home tab screen
  6. Go to bookmarks screen, ensure favorites are listed

Re-arrange favorites

  1. Add more favorites
  2. Go to a new tab
  3. Re-arrange favorites using drag and drop
  4. Go to bookmarks screen
  5. Ensure favorites list reflects previous changes (position)

Edit/Delete

  1. You can edit/delete (and undo) favorites
  2. Ensure those actions work from the home tab and from bookmarks screen

Autocomplete

  1. Having multiple favorites
  2. Go to URL bar and introduce any word part of favorites (title or URL)
  3. ensure favorites appear in the suggestions
  4. When browsing, focus the URL bar
  5. Ensure favorites are shown
  6. if you start typing, suggestions will appear instead
  7. clearing the input text will show favorites again

Favicon

  1. When adding websites as favorites, we know accept normal favicon (bitmap) and high-res favicons(apple-touch)
  2. Visit tesla.com and add it as favorite
  3. Ensure high-res favicon is persisted (previously, for this site, a low-res favicon was used)
  4. For websites without favicon (or that we haven't still stored any favicon) ensure a new default favicon appears
  5. Default favicon shows a letter and a random color

Database migration

  1. Check out develop
  2. run the app
  3. add some bookmarks
  4. Check out this branch
  5. run the app
  6. Ensure bookmarks are still present
  7. Ensure migration did not crash

Internal references:

Software Engineering Expectations
Technical Design Template

- Create entity
- Create DAO
- bookmarks listed with section title
- Added logic to display empty hint in favorites adapter
- Updated new UI elements to be displayed correctly and with expected copy
… favorites

- be able to reuse dialog
- be able to reuse command logic
- be able to reuse same listeners
- decouple from entity data objects from databases in UI
- if user moves the item, hide menu
# Conflicts:
#	app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
@cmonfortep cmonfortep marked this pull request as ready for review May 19, 2021 10:32

interface FavoritesRepository {
fun favoritesCountByDomain(domain: String): Int
fun favoritesObservable(): Single<List<SavedSite.Favorite>>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't be favoritesSingle ?

Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @cmonfortep, there is a lot of good stuff in here! As we discussed offline, there's a part regarding adding more RX code that we need to discuss internally. Other than that I've got a few suggestions that won't block the PR but I'd like you to consider.

fun whenReturnBookmarkSuggestionsThenPhraseIsURLBaseHost() {
whenever(mockAutoCompleteService.autoComplete("title")).thenReturn(Observable.just(emptyList()))
whenever(mockBookmarksDao.bookmarksObservable()).thenReturn(Single.just(listOf(BookmarkEntity(0, "title", "https://example.com"))))
whenever(mockFavoritesRepository.favoritesObservable()).thenReturn(Single.just(emptyList()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed on MM, let's discuss with the rest of the team if we want to refactor AutoComplete API and get rid of Rx or not.

cmonfortep added 13 commits May 24, 2021 10:38
# Conflicts:
#	app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
#	app/src/main/res/layout/include_new_browser_tab.xml
# Conflicts:
#	app/schemas/com.duckduckgo.app.global.db.AppDatabase/34.json
#	app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
#	app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
#	app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt
#	app/src/main/res/values/string-untranslated.xml
@cmonfortep cmonfortep merged commit 53b64f0 into develop May 25, 2021
@cmonfortep cmonfortep deleted the feature/cristian/favorites branch May 25, 2021 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants