Closed
Bug 1066546
Opened 10 years ago
Closed 10 years ago
Move new tab and overflow menu icons in the tabs layout to the top whilst in portrait mode
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: mhaigh, Assigned: mhaigh)
References
Details
Attachments
(2 files, 2 obsolete files)
9.98 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
3.84 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
Currently the new tabs icon and the overflow menu icon are based at the bottom of the tabs layout when in portrait mode, a leftover from the side bar currently implemented. This is implemented via android resource folder switching with the portrait and landscape layouts being drawn from different layout xml files (the layout-large-land-v11 folder having alternate resources for landscape). We need to move these UI elements back to the top whilst the browser is in portrait mode, whilst also keeping the existing functionality for when isNewTablet != true
Assignee | ||
Comment 1•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d1d3386cfab6
Assignee | ||
Comment 2•10 years ago
|
||
The long awaited patch!
Attachment #8498190 -
Flags: feedback?(lucasr.at.mozilla)
Comment 3•10 years ago
|
||
Comment on attachment 8498190 [details] [diff] [review] Move icons in the tabs layout to the top whilst in portrait mode Review of attachment 8498190 [details] [diff] [review]: ----------------------------------------------------------------- Could you please describe the changes in this patch? I think I need more context. ::: mobile/android/base/tabs/TabsPanel.java @@ +144,5 @@ > > + private void inflateLayout(Context context) { > + if(NewTabletUI.isEnabled(context)) { > + LayoutInflater.from(context).inflate(R.layout.tabs_panel_default, this); > + ViewStub header = (ViewStub)findViewById(R.id.tabs_panel_header_stub); Why do you need a viewstub for the header? To allow setting different attributes when in new tablet UI?
Attachment #8498190 -
Flags: feedback?(lucasr.at.mozilla)
Assignee | ||
Comment 4•10 years ago
|
||
The reason for the complexity here is that the tabs panel header and footer are both included files; without using the viewstub for the header the new tablet ui landscape will pick up the wrong layout file. We can solve this by inlining the header and footer in the relative files - I don't know why we didn't do this in the first place to be honest as there's a direct one to one correlation between the parent and header views.
Assignee | ||
Comment 5•10 years ago
|
||
Here's what this patch would look like if we inline the header and footer layouts in to the main tabs panel layout file
Attachment #8498190 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8499466 -
Flags: feedback?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8499468 -
Flags: feedback?(lucasr.at.mozilla)
Assignee | ||
Comment 7•10 years ago
|
||
Last one didn't include file name changes
Attachment #8499468 -
Attachment is obsolete: true
Attachment #8499468 -
Flags: feedback?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8499470 -
Flags: feedback?(lucasr.at.mozilla)
Comment 8•10 years ago
|
||
Comment on attachment 8499466 [details] [diff] [review] Part 1 - Inline header and footer layouts Review of attachment 8499466 [details] [diff] [review]: ----------------------------------------------------------------- Ah, now I see, makes sense ;-)
Attachment #8499466 -
Flags: feedback?(lucasr.at.mozilla) → review+
Comment 9•10 years ago
|
||
Comment on attachment 8499470 [details] [diff] [review] Part 2 - configure tabs panel to use correct layout for new tablet ui Review of attachment 8499470 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Just make sure the change to values-large-land-v11/layout.xml is working fine on phones and old/new tablet UIs. ::: mobile/android/base/resources/values-large-land-v11/layout.xml @@ -4,4 @@ > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > > <resources> > - <item type="layout" name="tabs_layout_item_view">@layout/tabs_item_row</item> Hmmm, isn't that supposed to point to tabs_item_cell in large-land?
Attachment #8499470 -
Flags: feedback?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/4a1cf6241e5f https://hg.mozilla.org/integration/fx-team/rev/9f9ab3840334
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4a1cf6241e5f https://hg.mozilla.org/mozilla-central/rev/9f9ab3840334
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
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
•