From 023392d94dfeed5001d0c5468f94dad05c1f2067 Mon Sep 17 00:00:00 2001 From: Tomasz Wiszkowski Date: Tue, 12 Sep 2023 01:07:35 +0000 Subject: [PATCH] Allow keyboard to be dismissed even if suggestions list doesn't scroll The suggestions list doesn't need to scroll for suggestions to show beneath the keyboard, even though it is most of then the case. In certain situations, additional suggestions may be available below the keyboard, but their number may not make the list scrollable. This change relaxes the current restriction, permitting users to dismiss the keyboard even if suggestions list doesn't show. (cherry picked from commit 4300766683f13216351343c1c40ac9b9d599fa4e) Change-Id: I478a17da65e01124b35d2e4a29f74538b00bc138 Fixed: 1479437 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4850684 Commit-Queue: Patrick Noland Auto-Submit: Tomasz Wiszkowski Reviewed-by: Patrick Noland Cr-Original-Commit-Position: refs/heads/main@{#1194189} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4857107 Commit-Queue: Tomasz Wiszkowski Bot-Commit: Rubber Stamper Commit-Queue: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5993@{#187} Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594} --- .../OmniboxSuggestionsDropdown.java | 5 +-- .../OmniboxSuggestionsDropdownUnitTest.java | 33 +++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java index 2b696a726395fa..0953252cd73839 100644 --- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java +++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java @@ -137,8 +137,9 @@ public int scrollVerticallyBy( // visibility), or // 2. the list is too short, and almost entirely fits on the screen, leaving at most // just a few pixels of content hiding under the keyboard. - // There's no need to dismiss the keyboard in any of these cases. - if (resultingDeltaY < requestedDeltaY) return resultingDeltaY; + // Note that the list may extend below the keyboard and still be non-scrollable: + // http://crbug/1479437 + // Otherwise decide whether keyboard should be shown or not. // We want to call keyboard up only when we know we reached the top of the list. // Note: the condition below evaluates `true` only if the scroll direction is "up", diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java index cb216907e0d7f5..74efeffe4346b4 100644 --- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java +++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java @@ -221,36 +221,41 @@ public void testScrollListener_keyboardShouldShowOnScrollToTop() { @Test @SmallTest - public void testScrollListener_notDismissingKeyboardWhenScrollDoesNotHappen() { + public void testScrollListener_dismissingKeyboardWhenScrollDoesNotHappen() { + // In some cases the list may be long enough to stretch below the keyboard, but not long + // enough to be scrollable. We want to dismiss the keyboard in these cases, too. mDropdown.setSuggestionDropdownScrollListener(mDropdownScrollListener); + mDropdown.setSuggestionDropdownOverscrolledToTopListener(mDropdownScrollToTopListener); // Pretend we're scrolling down (delta=10) but there is no content to move to (scroll=0). assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(0, 10)); - // Confirm that we're not hiding the keyboard. - verifyNoMoreInteractions(mDropdownScrollListener); + // Confirm that we're hiding the keyboard. + verify(mDropdownScrollListener).run(); // Pretend we're scrolling up now (delta=-10) but we're already on top and can't move. assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(0, -10)); // Confirm that we're not trying to show the keyboard. - verifyNoMoreInteractions(mDropdownScrollListener); + verify(mDropdownScrollToTopListener).run(); + + verifyNoMoreInteractions(mDropdownScrollListener, mDropdownScrollToTopListener); } @Test @SmallTest - public void testScrollListener_notDismissingKeyboardWhenTheListIsOnlyBarelyUnderTheKeyboard() { + public void testScrollListener_dismissingKeyboardWhenTheListIsOnlyBarelyUnderTheKeyboard() { mDropdown.setSuggestionDropdownScrollListener(mDropdownScrollListener); + mDropdown.setSuggestionDropdownOverscrolledToTopListener(mDropdownScrollToTopListener); - // We want to scroll by 10px, but there's only 1px of content. Don't hide the keyboard. - assertEquals(1, mListener.updateKeyboardVisibilityAndScroll(1, 10)); - verifyNoMoreInteractions(mDropdownScrollListener); - - // We want to scroll by 10px, but there's only 9px of content. Don't hide the keyboard. - assertEquals(9, mListener.updateKeyboardVisibilityAndScroll(9, 10)); - verifyNoMoreInteractions(mDropdownScrollListener); + // We want to scroll by 10px, but there's only 1px of slack. This means the suggestions list + // spans entirely under the keyboard. Hide the keyboard. + assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(1, 10)); + verify(mDropdownScrollListener).run(); - // But then, if we scroll back up, we likely should not ask for keyboard to show. + // Reset keyboard state. assertEquals(-9, mListener.updateKeyboardVisibilityAndScroll(-9, -10)); - verifyNoMoreInteractions(mDropdownScrollListener); + verify(mDropdownScrollToTopListener).run(); + + verifyNoMoreInteractions(mDropdownScrollListener, mDropdownScrollToTopListener); } @Test