Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For FNXV2-17067: always show home in background behind search dialog #20573

Merged

Conversation

eliserichards
Copy link
Contributor

@eliserichards eliserichards commented Jul 29, 2021

For FNXV2-17067
POC for FNXV2-17191 spike

  • Navigate to home before we show the search dialog (so home shows as the background)
  • Make sure the homescreen can be interacted with behind search dialog
  • Handle back presses
    • Navigate to wikipedia. Click on URL and see home. Press back button: home & search closes, be on wikipedia.
    • Navigate to wikipedia. Click on URL and see home. Click on Pocket. Press back button: should navigate to wikipedia, not home.
  • Handle normal URL navigation
    • Navigate to wikipedia. Click on URL and see home. Type in URL to go to google. Press back button: the back stack preserves wikipedia as the last destination (doesn't show homescreen)

Question: will this have any effect on the search widget?

Screenshots

20210803_145214.mp4
20210803_145249.mp4

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@eliserichards eliserichards requested review from a team as code owners July 29, 2021 20:36
}

@Test
fun handleToolbarClick_useNewSearchExperience() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We switched over to the "new" search experience a while ago, so this test is a dupe of the one above

@eliserichards eliserichards added this to In Development (WIP limit - 15) in Android Engineering Team Kanban board via automation Jul 29, 2021
@eliserichards eliserichards force-pushed the FNXV2-17067-home-in-background branch 2 times, most recently from dfd20ab to 991fd61 Compare August 2, 2021 22:14
@mergify
Copy link
Contributor

mergify bot commented Aug 3, 2021

This pull request has conflicts when rebasing. Could you fix it @eliserichards? 🙏

@eliserichards eliserichards force-pushed the FNXV2-17067-home-in-background branch 3 times, most recently from fd8dd3c to 591acec Compare August 3, 2021 19:44
@eliserichards eliserichards added the needs:review PRs that need to be reviewed label Aug 3, 2021
@eliserichards eliserichards moved this from In Development (WIP limit - 15) to Dev Complete (WIP limit - 5) in Android Engineering Team Kanban board Aug 3, 2021
@eliserichards
Copy link
Contributor Author

UI test: swipeToRemoveTabFromCollectionTest

Copy link
Contributor

@csadilek csadilek left a comment

Choose a reason for hiding this comment

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

Looks good! We need a quick profiling session though and be careful not to introduce a performance regression. See comment on the original fix below.

@mergify
Copy link
Contributor

mergify bot commented Aug 4, 2021

This pull request has conflicts when rebasing. Could you fix it @eliserichards? 🙏

@eliserichards
Copy link
Contributor Author

Flaky UI tests:

  • swipeToRemoveTabFromCollectionTest
  • verifyTabTrayNotShowingStateHalfExpanded

Both ran fine locally ✅

Android Engineering Team Kanban board automation moved this from Dev Complete (WIP limit - 5) to Done Aug 5, 2021
@eliserichards eliserichards reopened this Aug 5, 2021
Android Engineering Team Kanban board automation moved this from Done to In Development (WIP limit - 15) Aug 5, 2021
@eliserichards eliserichards added this to In progress in Home behind search via automation Aug 5, 2021
@@ -68,7 +68,7 @@
android:layout_height="match_parent"
android:clipChildren="false"
android:clipToPadding="false"
android:layout_marginBottom="48dp"
android:layout_marginBottom="88dp"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows collections to be seen on home behind the search dialog/toolbar

Copy link
Contributor

@csadilek csadilek left a comment

Choose a reason for hiding this comment

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

Did some manual testing again, and took another profile.

Looks good to 🚢 and gather some feedback. I think the follow-up to hide the keyboard while scrolling could be moved in soon. It will make this work better as we add more things to the home screen.

Android Engineering Team Kanban board automation moved this from In Development (WIP limit - 15) to Reviewer approved, ready for test (WIP limit - ?) Aug 6, 2021
@eliserichards
Copy link
Contributor Author

@Mergifyio rebase

Update tests to show home behind search dialog. Remove unused test.

Jump back in show all button is clickable behind search dialog

Recently saved bookmarks show all button is clickable behind search dialog
Handle keyboard in controllers instead of viewholders. Update tests.

Allow collections to be visible behind search dialog

Dismiss keyboard and search dialog with navigateUp instead of just dismissing the keyboard

Verify navigateUp in tests

Adding ignore for flaky UI test

Only resize home behind search dialog

Add ignore for collection intermittent test

Cleanup
@mergify
Copy link
Contributor

mergify bot commented Aug 9, 2021

Command rebase: success

Branch has been successfully rebased

@eliserichards eliserichards added pr:needs-landing-squashed PRs that are ready to land (squashed) [Will be merged by Mergify] and removed needs:review PRs that need to be reviewed labels Aug 9, 2021
@mergify mergify bot merged commit 7fdad97 into mozilla-mobile:main Aug 9, 2021
Android Engineering Team Kanban board automation moved this from Reviewer approved, ready for test (WIP limit - ?) to Done Aug 9, 2021
Home behind search automation moved this from In progress to Done Aug 9, 2021
@eliserichards eliserichards moved this from Done to Reviewer approved, ready for test (WIP limit - ?) in Android Engineering Team Kanban board Aug 9, 2021
@eliserichards eliserichards added the eng:qa:needed QA Needed label Aug 9, 2021
@eliserichards eliserichards linked an issue Aug 9, 2021 that may be closed by this pull request
9 tasks
@eliserichards eliserichards removed the eng:qa:needed QA Needed label Aug 9, 2021
@eliserichards eliserichards removed this from Reviewer approved, ready for test (WIP limit - ?) in Android Engineering Team Kanban board Aug 9, 2021
@eliserichards eliserichards removed this from Done in Home behind search Aug 9, 2021
czlucius pushed a commit to czlucius/fenix that referenced this pull request Aug 20, 2021
…ozilla-mobile#20573)

* Navigate to home on toolbar click. Handle back press from search dialog

Update tests to show home behind search dialog. Remove unused test.

Jump back in show all button is clickable behind search dialog

Recently saved bookmarks show all button is clickable behind search dialog

* Add feature flag

* Past explorations show all button is clickable behind search dialog

Handle keyboard in controllers instead of viewholders. Update tests.

Allow collections to be visible behind search dialog

Dismiss keyboard and search dialog with navigateUp instead of just dismissing the keyboard

Verify navigateUp in tests

Adding ignore for flaky UI test

Only resize home behind search dialog

Add ignore for collection intermittent test

Cleanup
czlucius pushed a commit to czlucius/fenix that referenced this pull request Aug 22, 2021
…ozilla-mobile#20573)

* Navigate to home on toolbar click. Handle back press from search dialog

Update tests to show home behind search dialog. Remove unused test.

Jump back in show all button is clickable behind search dialog

Recently saved bookmarks show all button is clickable behind search dialog

* Add feature flag

* Past explorations show all button is clickable behind search dialog

Handle keyboard in controllers instead of viewholders. Update tests.

Allow collections to be visible behind search dialog

Dismiss keyboard and search dialog with navigateUp instead of just dismissing the keyboard

Verify navigateUp in tests

Adding ignore for flaky UI test

Only resize home behind search dialog

Add ignore for collection intermittent test

Cleanup
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:needs-landing-squashed PRs that are ready to land (squashed) [Will be merged by Mergify]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show home in background behind search
3 participants