Closed
Bug 1232866
Opened 5 years ago
Closed 5 years ago
Establish clearer IA for panel list items
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: antlam, Assigned: liuche)
References
Details
Attachments
(5 files)
I'd like to clean up our home panel list items layouts. This will help establish a clearer information architecture and teach our users how to visually identify and recall the value behind these links consistently. Attaching preview of how it would look in History, and Synced tabs. Note: the favicons are a part of another bug (bug 1228680).
Reporter | ||
Comment 1•5 years ago
|
||
Attaching spec. Ideally, I'm hoping we could use this for most/all of our list items. That way, we can keep this fundamental UI element of "a link" consistent throughout our product. Broken down, it looks something like this: The space on the left becomes - "Where is this link going to send me?" The space to the left of the link underneath becomes - "What type of URL is this?" (RV, open on another device, etc) The right-most side becomes - "What's the status of this link?" (for now, it is mostly reserved for Bookmark)
Reporter | ||
Updated•5 years ago
|
Blocks: fennec-polish
Reporter | ||
Comment 2•5 years ago
|
||
Bookmark star - its blue!
(In reply to Anthony Lam (:antlam) from comment #1) > Ideally, I'm hoping we could use this for most/all of our list items. That > way, we can keep this fundamental UI element of "a link" consistent > throughout our product. I wonder if we should create new naming conventions or have a list of our "main" styles somewhere, to make finding and using these styles easier. Perhaps we should name the styles w/ antlam so he can tell us which to use by name, like we do with colors (e.g. fennec_ui_orange).
Reporter | ||
Updated•5 years ago
|
Blocks: bookmark-folders
Reporter | ||
Updated•5 years ago
|
Blocks: migrate-RL
Assignee | ||
Comment 6•5 years ago
|
||
I also verified that the divider is the right color and size.
Attachment #8724280 -
Flags: feedback?(alam)
Assignee | ||
Comment 7•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/36959/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36959/
Attachment #8724281 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 8•5 years ago
|
||
Comment on attachment 8724280 [details]
Screenshot: New layout
uh-MAZING!
Attachment #8724280 -
Flags: feedback?(alam) → feedback+
Updated•5 years ago
|
Assignee: nobody → liuche
Assignee | ||
Comment 9•5 years ago
|
||
I also see that we're using similar assets in bug 1227120, but bigger - maybe we should use the same assets between these two bugs.
Comment on attachment 8724281 [details] MozReview Request: Bug 1232866 - Establish clearer IA for panel list items. r=mcomella https://reviewboard.mozilla.org/r/36959/#review33871 r+ w/ a few clean-ups. ::: mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java:10 (Diff revision 1) > +import android.widget.ImageView; nit: I'd complain about import order but I've stopped caring (and wasting time on it) myself. Your call. :) ::: mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java:94 (Diff revision 1) > + setPadding(0, 0, (int) getResources().getDimension(R.dimen.page_row_padding_right), 0); Why not declare this in the places `TwoLinePageRow` is used in xml (it's corresponding style). ::: mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java:181 (Diff revision 1) > - private void setPageTypeIcon(int iconId) { > + private void showPageTypeIcon(boolean toShow) { This is no longer a `PageTypeIcon` but rather `BookmarkedIcon`, or similar. Can we rename the method here, as well as the view id? ::: mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java:232 (Diff revision 1) > if (bookmarkId == 0) { nit: `showPageTypeIcon(bookmarkId != 0)` ::: mobile/android/base/resources/layout/two_line_page_row.xml:29 (Diff revision 1) > - gecko:fadeWidth="30dp" > + gecko:fadeWidth="90dp" Why was this increased to 90dp? It seems extreme. ::: mobile/android/base/resources/layout/two_line_page_row.xml:33 (Diff revision 1) > style="@style/Widget.TwoLinePageRow.Url" If you're changing this to the `Faded...TextView`, you've got to remove the ellipsis attrs from the `TwoLinePageRow.Url` style. ::: mobile/android/base/resources/layout/two_line_page_row.xml:43 (Diff revision 1) > + <ImageView android:id="@+id/status_icon" nit: ws above & below ::: mobile/android/base/resources/layout/two_line_page_row.xml:44 (Diff revision 1) > + android:layout_width="@dimen/home_status_icon_size" nit: Any reason not to declare the dp right in the xml here? imo, it's generally better to write the values directly in the file if they're neither re-used nor change via different configuration.
Attachment #8724281 -
Flags: review?(michael.l.comella) → review+
Assignee | ||
Comment 11•5 years ago
|
||
https://reviewboard.mozilla.org/r/36959/#review33871 > Why was this increased to 90dp? It seems extreme. Anthony and I tried a few values in person and decided on this one.
Assignee | ||
Comment 12•5 years ago
|
||
https://reviewboard.mozilla.org/r/36959/#review34189
Assignee | ||
Comment 13•5 years ago
|
||
> ::: mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java:94
> (Diff revision 1)
> > + setPadding(0, 0, (int) getResources().getDimension(R.dimen.page_row_padding_right), 0);
>
> Why not declare this in the places `TwoLinePageRow` is used in xml (it's
> corresponding style).
A merge layout gets its top-level layout params stripped so you can't declare it in the xml. In this case you also can't declare it in the parent because the parent is a ListView and the params would apply to the whole list, not each list item.
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a0978b299ee5
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment 16•5 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #11) > https://reviewboard.mozilla.org/r/36959/#review33871 > > > Why was this increased to 90dp? It seems extreme. > > Anthony and I tried a few values in person and decided on this one. I echo that - with the bookmark star in place it sort of works, but for all other tabs it feels like wasted space to fade out so early - I'd rather have a few more title/URL characters visible.
Updated•3 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.
Description
•