Add clear button to search bar in search activity.

VERIFIED FIXED in Firefox 33

Status

Firefox for Android Graveyard
Search Activity
VERIFIED FIXED
4 years ago
5 months ago

People

(Reporter: eedens, Assigned: Margaret)

Tracking

unspecified
Firefox 33
All
Android

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
The search bar needs an "x" button; the behavior isn't completely specced yet. It will either:
 (1) Clear the search bar and keep the search bar focused
 (2) Remove focus from the search bar and send the user back to the dashboard.

As a first pass, let's add the "x" button along with a click handler.
(Assignee)

Comment 1

4 years ago
Created attachment 8456346 [details] [diff] [review]
Add clear button to search bar in search activity

PR here: https://github.com/ericedens/FirefoxSearch/pull/22
Attachment #8456346 - Flags: review?(eedens)
(Reporter)

Comment 2

4 years ago
Comment on attachment 8456346 [details] [diff] [review]
Add clear button to search bar in search activity

Review of attachment 8456346 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

::: mobile/android/search/java/org/mozilla/search/PostSearchFragment.java
@@ +60,5 @@
>      }
>  
>      public void startSearch(String query) {
>          setUrl(Constants.YAHOO_WEB_SEARCH_BASE_URL + Uri.encode(query));
>      }

Nit: I don't think we need to remove this line.

::: mobile/android/search/res/layout/search_auto_complete.xml
@@ +38,5 @@
> +                android:layout_width="26dp"
> +                android:layout_height="26dp"
> +                android:layout_gravity="right|center_vertical"
> +                android:layout_marginRight="3dp"
> +                android:background="@drawable/clear" />

Once the resources get added, you'll want to add the prefix of `search_` -- since the resources get copied into Fennec's resources directory, we use the prefix to avoid collisions.
Attachment #8456346 - Flags: review?(eedens) → review+
(Assignee)

Comment 3

4 years ago
(In reply to Eric Edens [:eedens] from comment #2)
> Comment on attachment 8456346 [details] [diff] [review]
> Add clear button to search bar in search activity
> 
> Review of attachment 8456346 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good!
> 
> ::: mobile/android/search/java/org/mozilla/search/PostSearchFragment.java
> @@ +60,5 @@
> >      }
> >  
> >      public void startSearch(String query) {
> >          setUrl(Constants.YAHOO_WEB_SEARCH_BASE_URL + Uri.encode(query));
> >      }
> 
> Nit: I don't think we need to remove this line.

Oops, I think that must have been generated by the grunt script.

> ::: mobile/android/search/res/layout/search_auto_complete.xml
> @@ +38,5 @@
> > +                android:layout_width="26dp"
> > +                android:layout_height="26dp"
> > +                android:layout_gravity="right|center_vertical"
> > +                android:layout_marginRight="3dp"
> > +                android:background="@drawable/clear" />
> 
> Once the resources get added, you'll want to add the prefix of `search_` --
> since the resources get copied into Fennec's resources directory, we use the
> prefix to avoid collisions.

Oh, good call. Also, not sure why the grunt script didn't move over the new resources into this patch... maybe a bug with the script? You'll see them added in the pull request.
(Assignee)

Comment 4

4 years ago
I just landed this on fx-team:
https://hg.mozilla.org/integration/fx-team/rev/51964ec6b084

Eric, can you merge the pull request?
(Reporter)

Comment 5

4 years ago
All done!

https://github.com/ericedens/FirefoxSearch/commits/master
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

4 years ago
Status: REOPENED → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/51964ec6b084
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33

Comment 7

4 years ago
Verified as fixed in build 34.0a1 (2014-08-04);
Device: Samsung Galaxy Nexus (Android 4.2.1).
status-firefox34: --- → verified

Updated

4 years ago
Status: RESOLVED → VERIFIED

Updated

5 months ago
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.