For #26957 - Exit search dialog when interacting with home fragment

[Alexandru2909]

Dismiss the search dialog when interacting with the home fragment. The code handling clicks from SearchDialogFragment for homescreen items has been removed.

All the buttons from the homescreen can be interacted with:

device-2022-11-15-161026.mp4

After the first click on the CFR, the search dialog is dismissed and the user can interact with the it.

device-2022-11-15-161156.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.

QA

• QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

1. Click on Checks at the top of the PR page.
2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
3. Click on the build-debug task.
4. Click on View task in Taskcluster in the new DETAILS section.
5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Fixes #26957

[gabrielluong]

Please consider doing a squash landing here. I don't believe there's any benefit for landing 11 commits for this fix and having that reflected in the history especially when someone else is trying to understand the fix while reviewing the git history. I do like the reverts as a nice touch in terms of a code review, but at the same time, I also ended up just looking at all the files change altogether during the review.

To be clear, I am not saying we shouldn't do multi-commits where it makes sense to do smaller single purpose commits. However, I am looking at this PR and thinking if we did land 11 commits where we are mostly removing code and only 5 lines in the SearchDialogFragment contains the core of the fix, it would seem a bit overkill.

[gabrielluong]

@Alexandru2909 Before landing this, could you also check in with @kbrosnan about the drawback with the keyboard being dismissed after clicking on the private mode button? Since this is causing a change in behaviour that you described, I am gonna add a do-not-land label since this could be unexpected to product. I am wondering if we could retrigger showing the keyboard if we clicked on the private mode button to maintain the current behaviour.

[mergify]

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

[mergify]

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

[mergify]

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

[gabrielluong]

@Alexandru2909 Before landing this, could you also check in with @kbrosnan about the drawback with the keyboard being dismissed after clicking on the private mode button? Since this is causing a change in behaviour that you described, I am gonna add a do-not-land label since this could be unexpected to product. I am wondering if we could retrigger showing the keyboard if we clicked on the private mode button to maintain the current behaviour.

@Alexandru2909 I saw the review request, but I wanted to know what was the resolution to this before I proceed. My concern prior was that this was introducing an unexpected behaviour (keyboard being dismissed after clicking on the private mode button) and this might have unintended consequences from a Product point of view (eg, we are making it harder to start searching or entering a URL after entering private mode). In my mind, the PR would be landable if we were able to maintain the existing behaviour with entering private mode.

[Alexandru2909]

@Alexandru2909 Before landing this, could you also check in with @kbrosnan about the drawback with the keyboard being dismissed after clicking on the private mode button? Since this is causing a change in behaviour that you described, I am gonna add a do-not-land label since this could be unexpected to product. I am wondering if we could retrigger showing the keyboard if we clicked on the private mode button to maintain the current behaviour.

@Alexandru2909 I saw the review request, but I wanted to know what was the resolution to this before I proceed. My concern prior was that this was introducing an unexpected behaviour (keyboard being dismissed after clicking on the private mode button) and this might have unintended consequences from a Product point of view (eg, we are making it harder to start searching or entering a URL after entering private mode). In my mind, the PR would be landable if we were able to maintain the existing behaviour with entering private mode.

Hi @gabrielluong ! in the last force-push I added the HomeFragmentLayout class which decides based on the clicked element from the homescreen if we should dismiss the search dialog. I haven't got any response from UX/product and with help from @Mugurell I worked on a method to maintain the current behavior for the PB button. This is the reason I re-requested a review.

[gabrielluong]

Hi @gabrielluong ! in the last force-push I added the HomeFragmentLayout class which decides based on the clicked element from the homescreen if we should dismiss the search dialog. I haven't got any response from UX/product and with help from @Mugurell I worked on a method to maintain the current behavior for the PB button. This is the reason I re-requested a review.

Okay great, just wanted to know that the current behaviour was maintained in your new push.

[gabrielluong]

Apologizes for delay - please don't hesitate to ping me for a review if it's taking awhile. Depending on how confident you're about this PR, I would be okay with landing this before the code freeze this Thursday. This would be a nice fix to get in. Otherwise, let's wait until after the merge date. Let's also consider squashing the merge. Thanks!