1306609 - Activity Stream: Highlights UX Polish

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

In order to make highlights look like in the mocks we need to:

- Remove "MORE" link next to highlights
- Add "source" name and icon to highlights
- Update padding/margin (currently there's no bottom margin at all)
- Update font sizes and colors

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

https://mozilla.invisionapp.com/share/6Q8CRFR8B#/screens/182981103

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Remove MORE link from activity stream panel.

Review commit: https://reviewboard.mozilla.org/r/83340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83340/

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Activity Stream: Update look and feel of highlights headline.

Review commit: https://reviewboard.mozilla.org/r/83342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83342/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Activity Stream: Update padding/margin of highlights cards.

Review commit: https://reviewboard.mozilla.org/r/83344/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83344/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Activity Stream: Update color of divider and circle page indicator.

Review commit: https://reviewboard.mozilla.org/r/83346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83346/

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Activity Stream highlights card: Update text colors.

Review commit: https://reviewboard.mozilla.org/r/83348/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83348/

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - BrowserProvider.getHighlights(): Reorder columns.

The UNION operator requires the two result sets to have the columns in the same order.

Review commit: https://reviewboard.mozilla.org/r/83602/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83602/

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Show source label next to highlights (Visited, Bookmarked).

Review commit: https://reviewboard.mozilla.org/r/83604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83604/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Remove unused HighlightsItem.

Going forward there will be just one highlights item/layout with an optional image.

Review commit: https://reviewboard.mozilla.org/r/83610/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83610/

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: Bug 1306609 - Rename CompactItem to HighlightsItem.

Review commit: https://reviewboard.mozilla.org/r/83612/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83612/

Comment 11

[Andrzej Hunt :ahunt]

Comment on attachment 8797699 [details]
Bug 1306609 - Remove MORE link from activity stream panel.

https://reviewboard.mozilla.org/r/83340/#review82526

Comment 12

[Andrzej Hunt :ahunt]

Comment on attachment 8797700 [details]
Bug 1306609 - Activity Stream: Update look and feel of highlights headline.

https://reviewboard.mozilla.org/r/83342/#review82530

Comment 13

[Andrzej Hunt :ahunt]

Comment on attachment 8797701 [details]
Bug 1306609 - Activity Stream: Update padding/margin of highlights cards.

https://reviewboard.mozilla.org/r/83344/#review82532

Comment 14

[Andrzej Hunt :ahunt]

Comment on attachment 8797702 [details]
Bug 1306609 - Activity Stream: Update color of divider and circle page indicator.

https://reviewboard.mozilla.org/r/83346/#review82534

Comment 15

[Andrzej Hunt :ahunt]

Comment on attachment 8797703 [details]
Bug 1306609 - Activity Stream highlights card: Update text colors.

https://reviewboard.mozilla.org/r/83348/#review82536

Comment 16

[Andrzej Hunt :ahunt]

Comment on attachment 8798000 [details]
Bug 1306609 - BrowserProvider.getHighlights(): Reorder columns.

https://reviewboard.mozilla.org/r/83602/#review82540

Comment 17

[Andrzej Hunt :ahunt]

Comment on attachment 8798005 [details]
Bug 1306609 - Remove unused HighlightsItem.

https://reviewboard.mozilla.org/r/83610/#review82554

Comment 18

[Andrzej Hunt :ahunt]

Comment on attachment 8798006 [details]
Bug 1306609 - Rename CompactItem to HighlightsItem.

https://reviewboard.mozilla.org/r/83612/#review82556

Comment 19

[Andrzej Hunt :ahunt]

Comment on attachment 8798001 [details]
Bug 1306609 - Show source label next to highlights (Visited, Bookmarked).

https://reviewboard.mozilla.org/r/83604/#review82560

::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java:108
(Diff revision 1)
>  
> +        private void updateSource(final Cursor cursor) {
> +            final boolean isBookmark = -1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID));
> +            final boolean isHistory = -1 != cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID));
> +
> +            if (isBookmark) {

My only concern here is what if we have a page that is bookmarked AND visited?

Our current algorithm doesn't pass this data through, but maybe we should file a bug to set a history index if available in the first part of getHighlights(), and possibly some similar code for bookmars?

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Andrzej Hunt :ahunt from comment #19)
> My only concern here is what if we have a page that is bookmarked AND
> visited?
> 
> Our current algorithm doesn't pass this data through, but maybe we should
> file a bug to set a history index if available in the first part of
> getHighlights(), and possibly some similar code for bookmars?

I think in practice it doesn't really matter: The page is in the list of highlights and whether we picked it from highlights or bookmarks is only a minor detail of the implementation.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b410783e3ca
Remove MORE link from activity stream panel. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/d7681748aa59
Activity Stream: Update look and feel of highlights headline. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/627e12f0132b
Activity Stream: Update padding/margin of highlights cards. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/9814ac6984ae
Activity Stream: Update color of divider and circle page indicator. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/775845e46f69
Activity Stream highlights card: Update text colors. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/62af2819dea2
BrowserProvider.getHighlights(): Reorder columns. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/07db42d3c4c7
Show source label next to highlights (Visited, Bookmarked). r=ahunt
https://hg.mozilla.org/integration/autoland/rev/b1a5eb9682ce
Remove unused HighlightsItem. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/9513fa05e63d
Rename CompactItem to HighlightsItem. r=ahunt

Comment 22

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7b410783e3ca
https://hg.mozilla.org/mozilla-central/rev/d7681748aa59
https://hg.mozilla.org/mozilla-central/rev/627e12f0132b
https://hg.mozilla.org/mozilla-central/rev/9814ac6984ae
https://hg.mozilla.org/mozilla-central/rev/775845e46f69
https://hg.mozilla.org/mozilla-central/rev/62af2819dea2
https://hg.mozilla.org/mozilla-central/rev/07db42d3c4c7
https://hg.mozilla.org/mozilla-central/rev/b1a5eb9682ce
https://hg.mozilla.org/mozilla-central/rev/9513fa05e63d