Closed
Bug 942889
Opened 10 years ago
Closed 10 years ago
Lists - Gallery layout
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: ibarlow, Assigned: oogunsakin)
References
Details
Attachments
(3 files, 2 obsolete files)
Designs will be posted shortly.
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Summary: Lists - Gallery layout (phone) → Lists - Gallery layout
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → oogunsakin
Reporter | ||
Comment 2•10 years ago
|
||
Here are some guidelines on how to scale and crop images in the gallery. One thing of note: let's use slightly larger default images on larger tablets, as it will look a little better that way.
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8368326 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8368326 [details] [diff] [review] bug-942889-gallery-layout.patch Review of attachment 8368326 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/resources/raw/fake_home_items.json @@ +9,5 @@ > "dataset_id": "fake-dataset", > "url": "http://example.com/second", > "title": "Second Example", > "description": "This is a second example" > +}, ] will fix
Comment 5•10 years ago
|
||
Comment on attachment 8368326 [details] [diff] [review] bug-942889-gallery-layout.patch Review of attachment 8368326 [details] [diff] [review]: ----------------------------------------------------------------- Looking good. Need to fix the style duplication. ::: mobile/android/base/home/PanelGridItemView.java @@ +48,4 @@ > } > > public void updateFromCursor(Cursor cursor) { } > +} \ No newline at end of file Unrelated, revert. ::: mobile/android/base/resources/values/colors.xml @@ +89,5 @@ > <color name="url_bar_shadow">#12000000</color> > > <color name="home_last_tab_bar_bg">#FFF5F7F9</color> > > + <color name="panel_grid_item_image_background">#D1D9E1</color> Is that a color that ibarlow suggested? ::: mobile/android/base/resources/values/styles.xml @@ +144,5 @@ > <item name="android:padding">5dip</item> > <item name="android:orientation">vertical</item> > </style> > > + <style name="Widget.PanelGridView" parent="Widget.GridView"> You don't need to replicate the styles on every configuration. The only thing that changes across configurations is the columnWidth. So, define the styles only once in here (values/styles.xml) and create a dimension value (i.e. in the dimens.xml files) that defines the value per configuration (180dp for phones, 250dp for tablets). Generally speaking: - Same layout, slightly different paddings, margins, alignment -> vary on styles - Same styles, slightly different diferent padding, margins -> vary on dimensions
Attachment #8368326 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
-Remove unnecessary styles -Square images
Attachment #8368326 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8368594 -
Flags: review?(lucasr.at.mozilla)
Comment 7•10 years ago
|
||
Comment on attachment 8368594 [details] [diff] [review] bug-942889-gallery-layout.patch Review of attachment 8368594 [details] [diff] [review]: ----------------------------------------------------------------- Looks nice. Just needs to fix the redundant bits. ::: mobile/android/base/resources/values-large-land-v11/dimens.xml @@ +5,5 @@ > > <resources> > > <dimen name="history_tab_widget_width">360dp</dimen> > + <dimen name="panel_grid_view_column_width">250dp</dimen> This one is redudant as you've already defined it in values-large-v11/dimens.xml. Remove. ::: mobile/android/base/resources/values-xlarge-land-v11/dimens.xml @@ +5,5 @@ > > <resources> > > <dimen name="history_tab_widget_width">480dp</dimen> > + <dimen name="panel_grid_view_column_width">250dp</dimen> Same here, values-large-v11/dimens.xml is enough to cover both small and large tablets on all orientations. Remove. ::: mobile/android/base/resources/values-xlarge-v11/dimens.xml @@ +11,5 @@ > <dimen name="tabs_counter_size">26sp</dimen> > <dimen name="tabs_panel_indicator_width">60dp</dimen> > <dimen name="tabs_panel_list_padding">8dip</dimen> > <dimen name="history_tab_widget_width">270dp</dimen> > + <dimen name="panel_grid_view_column_width">250dp</dimen> Ditto, remove. ::: mobile/android/base/resources/values/styles.xml @@ +148,5 @@ > + <style name="Widget.PanelGridView" parent="Widget.GridView"> > + <item name="android:paddingTop">0dp</item> > + <item name="android:stretchMode">columnWidth</item> > + <item name="android:layout_width">fill_parent</item> > + <item name="android:layout_height">fill_parent</item> nit: move layout_* to the top. @@ +155,5 @@ > + <item name="android:verticalSpacing">2dp</item> > + </style> > + > + <style name="Widget.PanelGridItemView"> > + <item name="android:orientation">vertical</item> PanelGridItemView is a FrameLayout, no orientation.
Attachment #8368594 -
Flags: review?(lucasr.at.mozilla) → feedback+
Comment 8•10 years ago
|
||
After having a look at the design specs, you should actually define the (columnwidth = 250dp) dimension in values-xlarge-v11/dimens.xml (instead of values-large-v11) as it only applies to large tablets.
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8368594 -
Attachment is obsolete: true
Attachment #8368635 -
Flags: review?(lucasr.at.mozilla)
Comment 10•10 years ago
|
||
Comment on attachment 8368635 [details] [diff] [review] Implement changes Review of attachment 8368635 [details] [diff] [review]: ----------------------------------------------------------------- Nice.
Attachment #8368635 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c7d8358b2d27
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
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
•