Adding option to move suggestion popup item text to omnibox.

Adding option to move suggestion popup item text to omnibox.

Currently there is no way to move suggestion item text to omnibox
to refine suggestion in a single step.
With this change an arrow will be appearing along with suggestion text
and url. On clicking the arrow corresponding suggestion text is moved
to omnibox and suggestions are refined accordingly.

BUG=

Committed: https://crrev.com/bfc368c441b4f318bc32ab889ff04faefebec379
Cr-Commit-Position: refs/heads/master@{#294542}

[ankit, 2014-09-10 07:15:00]

ankit2.kumar@samsung.com changed reviewers:
+ bauerb@chromium.org, tedchoc@chromium.org

[ankit, 2014-09-10 07:15:00]

@Bernhard,Ted
PTAL

[Bernhard Bauer, 2014-09-10 07:53:31]

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java
(right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java:49:
t1.setText(suggestionText);
I think you could directly inline this.

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java
(right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:144:
mUrlField);
Indent eight spaces w/r/t the preceding line?

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
File chrome/android/shell/res/layout/suggestion_item.xml (right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/suggestion_item.xml:13:
android:orientation="horizontal" >
No space before the ">".

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/suggestion_item.xml:23: <TextView
android:id="@+id/suggestion_item_label"
Indent these now they are inside of a new LinearLayout.

[ankit, 2014-09-10 09:15:52]

@Bernhard
PTAL comment and suggest.

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java
(right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java:49:
t1.setText(suggestionText);
On 2014/09/10 07:53:31, Bernhard Bauer wrote:
> I think you could directly inline this.

In onClickListener I am using the same variable suggestionText at 2 places, 
If I inline this then |mSuggestions.get(position).getDisplayText()|
be replicated at all places where suggestionText is being used.
Please suggest if inlining this is fine or it should be as is.

[Bernhard Bauer, 2014-09-10 10:40:35]

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java
(right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionArrayAdapter.java:49:
t1.setText(suggestionText);
On 2014/09/10 09:15:52, ankit wrote:
> On 2014/09/10 07:53:31, Bernhard Bauer wrote:
> > I think you could directly inline this.
> 
> In onClickListener I am using the same variable suggestionText at 2 places, 
> If I inline this then |mSuggestions.get(position).getDisplayText()|
> be replicated at all places where suggestionText is being used.
> Please suggest if inlining this is fine or it should be as is.

Oh, didn't see that. Okay, let's keep it as it is.

[ankit, 2014-09-10 11:16:56]

@Bernhard
PTAL new patch.
I have incorporated review comments.

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java
(right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/java/sr...
chrome/android/shell/java/src/org/chromium/chrome/shell/omnibox/SuggestionPopup.java:144:
mUrlField);
On 2014/09/10 07:53:31, Bernhard Bauer wrote:
> Indent eight spaces w/r/t the preceding line?

Done.

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
File chrome/android/shell/res/layout/suggestion_item.xml (right):

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/suggestion_item.xml:13:
android:orientation="horizontal" >
On 2014/09/10 07:53:31, Bernhard Bauer wrote:
> No space before the ">".

Done.

https://codereview.chromium.org/557263002/diff/1/chrome/android/shell/res/lay...
chrome/android/shell/res/layout/suggestion_item.xml:23: <TextView
android:id="@+id/suggestion_item_label"
On 2014/09/10 07:53:31, Bernhard Bauer wrote:
> Indent these now they are inside of a new LinearLayout.

Done.

[Bernhard Bauer, 2014-09-10 11:24:09]

LG, but please wait for Ted's review before landing. Thanks!

[ankit, 2014-09-10 11:57:03]

On 2014/09/10 11:24:09, Bernhard Bauer wrote:
> LG, but please wait for Ted's review before landing. Thanks!

@Ted
PTAL

[Ted C, 2014-09-11 20:25:59]

On 2014/09/10 11:57:03, ankit wrote:
> On 2014/09/10 11:24:09, Bernhard Bauer wrote:
> > LG, but please wait for Ted's review before landing. Thanks!
> 
> @Ted
> PTAL

lgtm -- but please be aware that we do not want to continue adding
new UI features to chrome shell and I will soon start rejecting
these kinds of changes unless there is a clear testing need.  This
is only supposed to be used for instrumenting chrome based tests and
this does not fit that.

[ankit, 2014-09-12 03:38:36]

On 2014/09/11 20:25:59, Ted C wrote:
> On 2014/09/10 11:57:03, ankit wrote:
> > On 2014/09/10 11:24:09, Bernhard Bauer wrote:
> > > LG, but please wait for Ted's review before landing. Thanks!
> > 
> > @Ted
> > PTAL
> 
> lgtm -- but please be aware that we do not want to continue adding
> new UI features to chrome shell and I will soon start rejecting
> these kinds of changes unless there is a clear testing need.  This
> is only supposed to be used for instrumenting chrome based tests and
> this does not fit that.

@Ted
Thanks for review and suggestion.
I will stop adding new UI features unless there is a clear testing need.

[ankit, 2014-09-12 03:38:42]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-12 03:39:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/557263002/20001

[commit-bot: I haz the power, 2014-09-12 04:43:22]

Committed patchset #2 (id:20001) as e4a16d293aca3a8ef30e607f0809956b519d651d

[commit-bot: I haz the power, 2014-09-12 04:45:09]

Patchset 2 (id:??) landed as
https://crrev.com/bfc368c441b4f318bc32ab889ff04faefebec379
Cr-Commit-Position: refs/heads/master@{#294542}