Closed Bug 739364 Opened 8 years ago Closed 7 years ago

[TABLET] Awesomebar Top Sites tab

Categories

(Firefox for Android :: General, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 16
Tracking Status
firefox15 --- verified
firefox16 --- verified
firefox17 --- verified
blocking-fennec1.0 --- -
fennec 14+ ---

People

(Reporter: ibarlow, Assigned: sriram)

References

Details

Attachments

(5 files, 1 obsolete file)

Attached image Mockup
Let's get the Awesomebar Top Sites (Frecency list) updated with the new UI.

A mockup is attached. More detailed specs and assets will follow soon.
-ing to keep triage lists clean
blocking-fennec1.0: --- → -
tracking-fennec: --- → 14+
Attached image UI Specs
The Top Sites list gives you quick access to sites you visit often, sites you’ve bookmarked, and sites you’ve visited recently. 

Tapping on the URL Bar opens the AwesomeBar and brings up the keyboard. Each AwesomeBar entry is distinguishable by the site’s favicon, the page title, and its URL. Any title and URL text that doesn’t fit in a single line is truncated in the center by an ellipsis (“...”). Tapping on any entry opens the page in the current tab.

More detailed information is included in the attached spec!
When a user arrows down a list with direction keys, should there be a separate highlight state color or should it visually look like a tap state?
Attached patch Patch (obsolete) — Splinter Review
*One deep breathe*
Finally! It's done. I've ensured to change all dependent layouts.
FlowLayout used for search suggestion is having problems on phones. I need to look into the code to fix it. I'll do it as a followup. (Basically it's not honouring minWidth attribute, hence the text might be chopped off at times).
Attachment #637253 - Flags: review?(mark.finkle)
Attached patch Patch 2/2Splinter Review
Being unable to hit reader button on a 3.5" screen, and getting frustrated over and over, I fixed determined to fix it, and fixed it! :D

This basically enlarges the target area to the entire height of the browsertoolbar, and adds padding to keep the icon at the required size.
Attachment #637567 - Flags: review?(mark.finkle)
Attached patch Patch (1/2)Splinter Review
Earlier we wanted a small little border below the "headers" in folders.
Now that's removed. So, I had removed the LinearLayout from awesomebar_header_row.xml.
That was crashing when we tried using Bookmarks. It's fixed in this patch.
Attachment #637253 - Attachment is obsolete: true
Attachment #637253 - Flags: review?(mark.finkle)
Attachment #637596 - Flags: review?(mark.finkle)
Comment on attachment 637596 [details] [diff] [review]
Patch (1/2)

>diff --git a/mobile/android/base/AwesomeBarTabs.java b/mobile/android/base/AwesomeBarTabs.java

>+    private void fixTabIndicator() {
>+        int selIndex = getCurrentTab();
>+        TabWidget tabWidget = getTabWidget();
>+        for (int i = 0; i < tabWidget.getTabCount(); i++) {
>+             if (i == selIndex)
>+                 continue;
>+
>+             if (i == (selIndex - 1))
>+                 tabWidget.getChildTabViewAt(i).getBackground().setLevel(1);
>+             else if (i == (selIndex + 1))
>+                 tabWidget.getChildTabViewAt(i).getBackground().setLevel(2);
>+             else
>+                 tabWidget.getChildTabViewAt(i).getBackground().setLevel(0);
>+        }
>+
>+        if (selIndex == 0)
>+            findViewById(R.id.tab_widget_left).getBackground().setLevel(1);
>+        else
>+            findViewById(R.id.tab_widget_left).getBackground().setLevel(0);
>+
>+        if (selIndex == (tabWidget.getTabCount() - 1))
>+            findViewById(R.id.tab_widget_right).getBackground().setLevel(2);
>+        else
>+            findViewById(R.id.tab_widget_right).getBackground().setLevel(0);
>+    }

If I understand this function, you are styling the selected tab, using the adjacent tabs to show the "curves" if needed.
In that case, lets rename the function to "styleSelectedTab()"
Attachment #637596 - Flags: review?(mark.finkle) → review+
Attachment #637567 - Flags: review?(mark.finkle) → review+
This is same as Patch (1/2). This has been made ready for inbound.

Note: The list styling is still available in styles.xml. However they are not applied properly, as with the changes in Bug 759041, the lists are created in Java, which doesn't take the styles into consideration.
Attachment #638027 - Flags: review?(mark.finkle)
Comment on attachment 638027 [details] [diff] [review]
Patch (1.5/2): For Inbound

This looks fine, but file a bug to fix the styling on trunk.
Attachment #638027 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/a92bb4e1842d
https://hg.mozilla.org/mozilla-central/rev/28d34d5f50ae
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Depends on: 770597
Comment on attachment 637567 [details] [diff] [review]
Patch 2/2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: The AwesomeScreen will not be tablet optimized.
Testing completed (on m-c, etc.): Landed on m-c on 07/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: -

Note: The UI of AwesomeScreen in current m-c feels broken as the styles aren't applied due to a change in AwesomeBar that landed. This patch is for aurora -- which doesn't have the AwesomeBar changes, and tested to work fine.
Attachment #637567 - Flags: approval-mozilla-aurora?
Comment on attachment 637596 [details] [diff] [review]
Patch (1/2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): New UI
User impact if declined: The AwesomeScreen will not be tablet optimized.
Testing completed (on m-c, etc.): Landed on m-c on 07/02
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: -

Note: The UI of AwesomeScreen in current m-c feels broken as the styles aren't applied due to a change in AwesomeBar that landed. This patch is for aurora -- which doesn't have the AwesomeBar changes, and tested to work fine.
Attachment #637596 - Flags: approval-mozilla-aurora?
Depends on: 771380
(In reply to Sriram Ramasubramanian [:sriram] from comment #12)
> User impact if declined: The AwesomeScreen will not be tablet optimized.

Does this give us style parity between tablets and phones on Aurora 15? I just want to make sure they won't be visually different when we release.
This is phone and tablet ready -- as per the specifications. They both will be visually similar (with some optimizations based on the form factor).
Comment on attachment 637567 [details] [diff] [review]
Patch 2/2

[Triage Comment]
This is the tablet interface that we've been testing on m-c, so we need to uplift. We also want phone UI parity, so we'll take the changes for that form factor as well. Approved for Aurora 15.
Attachment #637567 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #637596 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 767368
Search Suggestions are not in Aurora. Can I just bit-trot the patch and uplift mine?
Landed with "Search suggestions" related code removed.
Blocks: 586885
Assignee: nobody → sriram
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.