Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(782)

Issue 517403003: Making suggestion popup scrollable in landscape mode (Closed)

Created:
6 years, 3 months ago by ankit
Modified:
6 years, 3 months ago
Reviewers:
Bernhard Bauer, Ted C
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Making suggestion popup scrollable in landscape mode Setting height for Suggestion popup correctly for portrait as well as landscape mode so that it is scrollable and does not overlap with IME. BUG=409619 Committed: https://crrev.com/3d52a11f9d247d937c74983c32b95fac39456945 Cr-Commit-Position: refs/heads/master@{#292917}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Modified code as suggested. #

Total comments: 7

Patch Set 3 : Refactored code as per review comments #

Patch Set 4 : Applying margin if constrained by app rect. #

Patch Set 5 : Corrected comment line position. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -1 line) Patch
M chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java View 1 2 3 4 6 chunks +23 lines, -1 line 0 comments Download

Messages

Total messages: 14 (2 generated)
ankit
PTAL
6 years, 3 months ago (2014-09-01 12:58:58 UTC) #2
Bernhard Bauer
https://codereview.chromium.org/517403003/diff/1/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java File chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java (right): https://codereview.chromium.org/517403003/diff/1/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode110 chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:110: int height = dropDownItemHeight; This value isn't used (and ...
6 years, 3 months ago (2014-09-01 13:45:11 UTC) #3
ankit
@Bernhard PTAL new patch. https://codereview.chromium.org/517403003/diff/1/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java File chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java (right): https://codereview.chromium.org/517403003/diff/1/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode110 chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:110: int height = dropDownItemHeight; On ...
6 years, 3 months ago (2014-09-02 04:20:12 UTC) #4
Bernhard Bauer
https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java File chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java (right): https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode113 chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:113: int height = dropDownItemHeight; This assignment is unnecessary. https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode115 ...
6 years, 3 months ago (2014-09-02 08:54:16 UTC) #5
ankit
@Bernhard PTAL https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java File chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java (right): https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode113 chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:113: int height = dropDownItemHeight; On 2014/09/02 08:54:15, ...
6 years, 3 months ago (2014-09-02 09:23:04 UTC) #6
Bernhard Bauer
https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java File chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java (right): https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java#newcode120 chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:120: (popupHeight - dropDownItemHeight) : popupHeight; On 2014/09/02 09:23:04, ankit ...
6 years, 3 months ago (2014-09-02 09:31:02 UTC) #7
ankit
On 2014/09/02 09:31:02, Bernhard Bauer wrote: > https://codereview.chromium.org/517403003/diff/20001/chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java > File > chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java > (right): > ...
6 years, 3 months ago (2014-09-02 09:51:48 UTC) #8
Bernhard Bauer
LGTM!
6 years, 3 months ago (2014-09-02 10:10:52 UTC) #9
ankit
On 2014/09/02 10:10:52, Bernhard Bauer wrote: > LGTM! Thanks.
6 years, 3 months ago (2014-09-02 10:12:39 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ankit2.kumar@samsung.com/517403003/80001
6 years, 3 months ago (2014-09-02 10:13:48 UTC) #12
commit-bot: I haz the power
Committed patchset #5 (id:80001) as 05f8975e73c14ebe264873bb5464579b9962d620
6 years, 3 months ago (2014-09-02 13:56:07 UTC) #13
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:18:13 UTC) #14
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/3d52a11f9d247d937c74983c32b95fac39456945
Cr-Commit-Position: refs/heads/master@{#292917}

Powered by Google App Engine
This is Rietveld 408576698