Closed Bug 1041026 Opened 9 years ago Closed 9 years ago

Use a single click listener for activating search mode


(Firefox for Android Graveyard :: Search Activity, defect)

Not set


(Not tracked)

Firefox 34


(Reporter: eedens, Assigned: Margaret)




(1 file)

Followup from bug 1022102. 

I see a couple of options

1) Have the parent group intercept touches and decide whether to deliver to the child editText. If it's RUNNING, deliver. If it's WAITING, don't deliver. Ref:

2) Increase the touchable area with a TouchDelegate

Summary: Use a single click listener for activating → Use a single click listener for activating search mode
Feedback for :

Overall, I think this is a good direction. :)

I feel like there are some overlapping concerns between SearchFragment and SearchBar that might make debugging tricky. At a superficial level, I see calls to 'onSearch' in both classes, and it takes awhile to unravel the code path. More importantly, there could be subtle side effects that would be a surprise for the uninitiated.  For example, calling searchBar.setText will cause a couple of events:
  - A change listener for the editText will fire
  - The editText will call filter on the SearchListener
  - The filter spawns a network request 
  -  etc ..

I'd recommend focusing the SearchBar into something less search-y. Suppose it's called ClearableEditText, and has this interface:

  setText  // Probably include a boolean param to decide whether onChange should be fired
  clearText // Maybe redundant. Again, if included, include the boolean param
  onChange  // Text changed
  onClear  // Text has been cleared; could be consolidated with onChange
  onCancel  // User wants to exit the search input (eg: clicking down/back on their keyboard, or clicking some cancel button)
  onSubmit // User wants to complete the search (eg: clicking OK button on keyboard, or clicking some submit button)

Given this generalized interface, SearchFragment binds each event to the proper service/thread/listener/activity/whatever. For example, in onSubmit:
  - Transition from RUNNING to WAITING
  - Send the search query to the parent activity.
I want to base my polish changes on top of this, so let's work towards landing it.

I added a comment about the onInterceptTouchEvent implementation. Let me know if there are other spots that you think need clarification.
Attachment #8460536 - Flags: review?(eedens)
Comment on attachment 8460536 [details] [diff] [review]
Move search bar logic into custom view

Review of attachment 8460536 [details] [diff] [review]:

Looks good to me!
Attachment #8460536 - Flags: review?(eedens) → review+
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.