Closed Bug 871522 Opened 11 years ago Closed 11 years ago

Factor out EditText bits from BrowserToolbar into a CustomView

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect)

Product:
Firefox for Android Graveyard
Component:
Awesomescreen
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Firefox 28

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(3 files)

Encapsulate isEditing assignment into a setter (r=sriram)
(deleted), patch
sriram
: review+
Details | Diff | Splinter Review
Remove unnecessary long press listener from toolbar (r=sriram)
(deleted), patch
sriram
: review+
Details | Diff | Splinter Review
Factor out EditText handling into ToolbarEditText (r=sriram)
(deleted), patch
sriram
: review+
Details | Diff | Splinter Review
Right now, all the EditText handling from awesomescreen was moved directly into BrowserToolbar which is non-ideal.
Maybe it will be better to turn the layout containing the EditText and the go Button to a custom view (as opposed to just the EditText part) because the EditText callbacks have to update the go button in certain cases.
Whiteboard: good-first-bug-fig
Whiteboard: good-first-bug-fig → good first bug-fig
This isn't actually a [good first bug] in the traditional sense, good-first-bug-fig was a tag we were using for our new about:home work. However, this could probably be a mentor bug. Lucas, do you want to be the mentor here?
Flags: needinfo?(lucasr.at.mozilla)
Whiteboard: good first bug-fig
Sure.
Flags: needinfo?(lucasr.at.mozilla)
Whiteboard: [mentor=lucasr][lang=java]
Doing this as part of major refactoring in toolbar. Taking.
Assignee: nobody → lucasr.at.mozilla
Whiteboard: [mentor=lucasr][lang=java]
Attachment #831591 - Flags: review?(sriram)
Attachment #831592 - Flags: review?(sriram)
Attachment #831593 - Flags: review?(sriram)
Attachment #831591 - Flags: review?(sriram) → review+
Comment on attachment 831592 [details] [diff] [review] Remove unnecessary long press listener from toolbar (r=sriram) Review of attachment 831592 [details] [diff] [review]: ----------------------------------------------------------------- I think this is necessary. Didn't we add this to avoid showing context menu, but selecting all text, when long pressing in editing mode?
Attachment #831592 - Flags: review?(sriram) → review-
Attachment #831593 - Flags: review?(sriram) → review+
(In reply to Sriram Ramasubramanian [:sriram] from comment #8) > Comment on attachment 831592 [details] [diff] [review] > Remove unnecessary long press listener from toolbar (r=sriram) > > Review of attachment 831592 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think this is necessary. Didn't we add this to avoid showing context menu, > but selecting all text, when long pressing in editing mode? This doesn't seem necessary anymore, can't really see why to be honest. Long clicking on entry selects all text even without the long click handler. In any case, note how the current long click handler always returns false (which makes it kinda pointless).
Flags: needinfo?(sriram)
Comment on attachment 831592 [details] [diff] [review] Remove unnecessary long press listener from toolbar (r=sriram) Requesting review after clarification.
Attachment #831592 - Flags: review- → review?(sriram)
Sounds good.
Flags: needinfo?(sriram)
Attachment #831592 - Flags: review?(sriram) → review+
No longer depends on: 971355
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: