Skip to content
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

Use New Tab Page implementation without customization features #3453

Merged
merged 15 commits into from
Oct 23, 2024

Conversation

dus7
Copy link
Contributor

@dus7 dus7 commented Oct 18, 2024

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

Description:

Uses NewTabPageController as a home controller, but removes new tab page customization capabilities based on newTabPageSections feature flag state. Existing features of Home Screen should remain unchanged.

Summary of changes done in this PR:

  • SimpleNewTabPageView now serving as a view for new tab page without customization features. Created based on the fully featured NewTabPageView.
  • Favorites placeholders and add button are not visible when customization flag is disabled.
  • Report action is not added to the browsing menu when link is missing. This prevents showing option which performs no action in case menu is shown on NTP. (cc @afterxleep)
  • Favorites data source adapter is listening for display mode changes and updates favorites accordingly.
  • Current (old) Dax Onboarding is integrated with the New Tab Page. (cc @alessandroboron)

Steps to test this PR:

Dax onboarding

  1. Install fresh app
  2. Go through onboarding, make sure Dax dialogs are shown properly on empty tab.

Basic functionality

  1. Make sure NTP flag is disabled in debug menu.
  2. Without favorites, Dax logo should be visible.
  3. Add some favorites.
  4. It should be possible to long press to edit/remove and drag and drop to reorder.

Toolbar menu items

  1. Check toolbar displays bookmarks icon when NTP sections flag disabled.
  2. Enable NTP sections flag in debug menu.
  3. Reopen new tab page
  4. Ensure browsing menu with shortcuts is available.

Sync

  1. Enable sync on two devices.
  2. Verify updates for favorites are visible.

Definition of Done (Internal Only):

Copy Testing:

  • Use of correct apostrophes in new copy, ie rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 15
  • iOS 16
  • iOS 17

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

Copy link

github-actions bot commented Oct 18, 2024

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against d061323

@alessandroboron
Copy link
Contributor

@dus7 I went through the onboarding and the dax dialogs are shown properly. Thanks for taking care of this!

@dus7 dus7 marked this pull request as ready for review October 18, 2024 14:35
@dus7 dus7 requested a review from brindy October 18, 2024 14:40
Copy link
Contributor

@brindy brindy left a comment

Choose a reason for hiding this comment

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

LGTM

had some problems with sync at first, but worked subsequently so some kind of race condition not related to this PR.

Also favorites favicons didn't update in real time after the sync, but not too bothered by that.

@dus7 dus7 merged commit ec52eaa into main Oct 23, 2024
67 of 69 checks passed
@dus7 dus7 deleted the mariusz/ntp-cleanup branch October 23, 2024 15:31
@dus7 dus7 mentioned this pull request Oct 25, 2024
14 tasks
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