Closed Bug 1041026 Opened 7 years ago Closed 7 years ago
Use a single click listener for activating search mode
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: https://developer.android.com/training/gestures/viewgroup.html 2) Increase the touchable area with a TouchDelegate Ref: https://developer.android.com/training/gestures/viewgroup.html
Summary: Use a single click listener for activating → Use a single click listener for activating search mode
Feedback for https://github.com/leibovic/FirefoxSearch/commit/5bcde84c3241f808471bae3eb40e80f6319ec10e : 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: ``` ClearableEditText Methods: setText // Probably include a boolean param to decide whether onChange should be fired clearText // Maybe redundant. Again, if included, include the boolean param setActive Events: 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+
https://hg.mozilla.org/integration/fx-team/rev/4ebd3df15b4c I updated the PR here for you to merge: https://github.com/ericedens/FirefoxSearch/pull/29
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
You need to log in before you can comment on or make changes to this bug.