Closed
Bug 996850
Opened 10 years ago
Closed 10 years ago
Top sites: Empty tile behavior
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 31
People
(Reporter: liuche, Assigned: liuche)
References
Details
Attachments
(4 files, 2 obsolete files)
This might be moot with the new Tiles behavior, but a couple of things I noticed: - Don't show a context menu for an empty tile - none of these options (Share, Add to home screen, Edit) are useful - From the edit dialog, if the user doesn't enter anything and hits enter, the tile should revert back to "Add a site" instead of "user-entered:"
Comment 1•10 years ago
|
||
Now that we support actionmodes across all Android versions, I wonder if its worth using that here instead of context menus like we originally tried.
Updated•10 years ago
|
Component: General → Awesomescreen
Assignee | ||
Comment 2•10 years ago
|
||
Assignee: nobody → liuche
Status: NEW → ASSIGNED
Attachment #8407718 -
Flags: review?(lucasr.at.mozilla)
Attachment #8407718 -
Flags: feedback?(oogunsakin)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8407720 -
Flags: review?(lucasr.at.mozilla)
Attachment #8407720 -
Flags: feedback?(oogunsakin)
Assignee | ||
Comment 4•10 years ago
|
||
Ian, we should allow users to hit "enter" if they don't have any text entered in the Top Sites "Edit" dialog. What do you think about this Toast text?
Flags: needinfo?(ibarlow)
Comment 5•10 years ago
|
||
Comment on attachment 8407718 [details] [diff] [review] Part 1: Don't show context menu for empty tile Review of attachment 8407718 [details] [diff] [review]: ----------------------------------------------------------------- looks good to me!
Attachment #8407718 -
Flags: feedback?(oogunsakin) → feedback+
Comment 6•10 years ago
|
||
Comment on attachment 8407720 [details] [diff] [review] Part 2: Don't allow empty "Edit" entries Review of attachment 8407720 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Just a small edge case. ::: mobile/android/base/home/PinSiteDialog.java @@ +110,5 @@ > > // If the user manually entered a search term or URL, wrap the value in > // a special URI until we can get a valid URL for this bookmark. > final String text = mSearch.getText().toString(); > + if (!TextUtils.isEmpty(text)) { Don't believe this handles the case where a user enters only whitespace.
Attachment #8407720 -
Flags: feedback?(oogunsakin) → feedback+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Sola Ogunsakin [:sola] from comment #6) > Comment on attachment 8407720 [details] [diff] [review] > Part 2: Don't allow empty "Edit" entries > > Review of attachment 8407720 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good! Just a small edge case. > > ::: mobile/android/base/home/PinSiteDialog.java > @@ +110,5 @@ > > > > // If the user manually entered a search term or URL, wrap the value in > > // a special URI until we can get a valid URL for this bookmark. > > final String text = mSearch.getText().toString(); > > + if (!TextUtils.isEmpty(text)) { > > Don't believe this handles the case where a user enters only whitespace. Good catch!
Attachment #8407720 -
Attachment is obsolete: true
Attachment #8407720 -
Flags: review?(lucasr.at.mozilla)
Attachment #8407934 -
Flags: review?(lucasr.at.mozilla)
Comment 8•10 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #4) > Created attachment 8407722 [details] > Screenshot: Edit text toast > > Ian, we should allow users to hit "enter" if they don't have any text > entered in the Top Sites "Edit" dialog. What do you think about this Toast > text? I think i would prefer to disable the Enter/Go button, to be honest. Showing a toast feels like a bit of a strange workaround.
Flags: needinfo?(ibarlow)
Assignee | ||
Updated•10 years ago
|
Attachment #8407934 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 9•10 years ago
|
||
Ian, sorry, I'm not sure there's a way to enable/disable keys on the soft keyboard on the fly based on what's been typed - we can only intercept the enter key when it's been pressed and then "do something". I tried a few things (and also googled a bit), but without success. (Keep in mind this dialog is different from the edit bookmark or history dialog in that it doesn't have a "Go" button, just a text box and any search entries that match what's been typed, and solely relies on keyboard input.) I can't think of any Android apps off the top of my head that do that - do you know of any?
Flags: needinfo?(ibarlow)
Comment 10•10 years ago
|
||
Comment on attachment 8407718 [details] [diff] [review] Part 1: Don't show context menu for empty tile Review of attachment 8407718 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: mobile/android/base/home/TopSitesGridItemView.java @@ +110,5 @@ > /** > + * @return true, if this view has no content to show. > + */ > + public boolean isEmpty() { > + return mIsEmpty; mIsEmpty is going away in bug 997660. Added a dependency as a note to self to rebase my patches. ::: mobile/android/base/home/TopSitesGridView.java @@ +124,5 @@ > @Override > public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id) { > Cursor cursor = (Cursor) parent.getItemAtPosition(position); > > + TopSitesGridItemView gridView = (TopSitesGridItemView) view; nit: rename gridView to item for clarity. @@ +125,5 @@ > public boolean onItemLongClick(AdapterView<?> parent, View view, int position, long id) { > Cursor cursor = (Cursor) parent.getItemAtPosition(position); > > + TopSitesGridItemView gridView = (TopSitesGridItemView) view; > + if (cursor == null || gridView.isEmpty()) { Note to self: this will become item.getType() == TopSites.TYPE_BLANK.
Attachment #8407718 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 11•10 years ago
|
||
After further investigation, I don't think there is a way to disable a specific key on the keyboard. We do switch keyboard IME types in ToolbarEditText, but I don't think there's a keyboard that lacks an "action" key (be that return, "Done", or "Next"), so I don't think there's a workaround for disabling the return key.
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8407934 [details] [diff] [review] Part 2: Don't allow empty "Edit" entries v2 Ian, is there something else that you'd like to see instead, other than a toast? I haven't been able to find something that fits what you're asking for regarding the keyboard. Lucas, if you have another suggestion, that's welcome too! I tried actionNone, flagNoEnterAction, and flagNoAccessoryAction in imeOptions, but none of them remove the action button.
Attachment #8407934 -
Flags: review?(lucasr.at.mozilla)
Comment 13•10 years ago
|
||
Comment on attachment 8407934 [details] [diff] [review] Part 2: Don't allow empty "Edit" entries v2 Review of attachment 8407934 [details] [diff] [review]: ----------------------------------------------------------------- Yep.
Attachment #8407934 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 14•10 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #12) > Comment on attachment 8407934 [details] [diff] [review] > Part 2: Don't allow empty "Edit" entries v2 > > Ian, is there something else that you'd like to see instead, other than a > toast? I haven't been able to find something that fits what you're asking > for regarding the keyboard. I guess my only other suggestion would be that if there isn't any text in the url bar and the user hits Go, nothing should happen. Not even a toast.
Flags: needinfo?(ibarlow)
Assignee | ||
Comment 15•10 years ago
|
||
Removed displaying toast and strings, carrying over r+.
Attachment #8407934 -
Attachment is obsolete: true
Attachment #8410438 -
Flags: review+
Assignee | ||
Comment 16•10 years ago
|
||
Part 1: https://hg.mozilla.org/integration/fx-team/rev/a4891371e28e Part 2 and backout (for Toast): https://hg.mozilla.org/integration/fx-team/rev/fcd1191c12b5 https://hg.mozilla.org/integration/fx-team/rev/3f2032f4dad7 Part 2 reland: https://hg.mozilla.org/integration/fx-team/rev/4c8ee55dbdd1
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → Firefox 31
https://hg.mozilla.org/mozilla-central/rev/a4891371e28e https://hg.mozilla.org/mozilla-central/rev/4c8ee55dbdd1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•3 years 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.
Description
•