See attached image: The style of highlights changed a little bit. Additionally we should make sure to use the measurements defined in the spec.

Comment on attachment 8800688 [details] Bug 1308525 - Update activity stream panel to follow new UX mocks. https://reviewboard.mozilla.org/r/85242/#review84614 LGTM, I'll read up a bit more on layouts and look through this stuff again, but I trust that it all works :) ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStream.java:70 (Diff revision 1) > * Extract a label from a URL to use in Activity Stream. > * > * This method implements the proposal from this desktop AS issue: > * https://github.com/mozilla/activity-stream/issues/1311 > */ > - public static String extractLabel(String url) { > + public static String extractLabel(String url, boolean usePath) { Perhaps if usePath=true is pretty much your "default" value, adding a `extractLabel(String url) { extractLabel(url, true); }` method is worth it? ::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java:87 (Diff revision 1) > > + @Override > + protected void onSizeChanged(int w, int h, int oldw, int oldh) { > + super.onSizeChanged(w, h, oldw, oldh); > + > + int tiles = (w - tileMargin) / (desiredTileWidth + tileMargin); That's nice, cleaner than trying to explicitely account for margins on either side of the tile :) ::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java:90 (Diff revision 1) > + super.onSizeChanged(w, h, oldw, oldh); > + > + int tiles = (w - tileMargin) / (desiredTileWidth + tileMargin); > + > + if (tiles < MINIMUM_TILES) { > + tiles = MINIMUM_TILES; If there isn't enough space, wouldn't you want to have less tiles? ::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java:98 (Diff revision 1) > + } else if (tiles > MAXIMUM_TILES) { > + tiles = MAXIMUM_TILES; > + > + // Use the remaining space as padding > + int needed = tiles * (desiredTileWidth + tileMargin) + tileMargin; > + int padding = (w - needed) / 2; Couldn't this end up being a negative value, depending on what your desiredTileWidth is in relation to w? ::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/StreamItem.java:86 (Diff revision 1) > + private int tilesMargin; > > public HighlightItem(View itemView) { > super(itemView); > + > + tilesMargin = (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, 10, itemView.getContext().getResources().getDisplayMetrics()); Did you mean to use `activity_stream_base_margin` value? ::: mobile/android/base/resources/layout/activity_stream_card_history_item.xml:49 (Diff revision 1) > + > + <TextView > + android:id="@+id/page" > + android:layout_width="match_parent" > + android:layout_height="wrap_content" > + tools:text="twitter" placeholder? ::: mobile/android/base/resources/layout/activity_stream_card_history_item.xml:75 (Diff revision 1) > android:textStyle="bold" > android:textColor="#ff000000" > - tools:text="Descriptive title of a page..." /> > + android:layout_below="@id/page" > + android:layout_toLeftOf="@id/menu" > + android:layout_toStartOf="@id/menu" > + tools:text="Descriptive title of a page that is veeeeeeery long - maybe even too long?" /> placeholder? ::: mobile/android/base/resources/layout/activity_stream_topsites_card.xml:45 (Diff revision 1) > android:gravity="center" > android:lines="1" > android:padding="4dp" > + android:textSize="12sp" > android:textColor="@android:color/black" > - tools:text="Lorem Ipsum here is a title" > + tools:text="Lorem Ipsum here is a title" /> I guess there are a lot of placeholders for now :) ::: mobile/android/base/resources/values/dimens.xml:222 (Diff revision 1) > > <item name="notification_media_cover" type="dimen">128dp</item> > + > + <item name="activity_stream_base_margin" type="dimen">10dp</item> > + <item name="activity_stream_desired_tile_width" type="dimen">90dp</item> > + <item name="activity_stream_desired_tile_height" type="dimen">70dp</item> Proportionally tile height/width are a bit off from what the spec is (ratios of 0.77 vs 0.89). I guess that's on purpose?

Comment on attachment 8800688 [details] Bug 1308525 - Update activity stream panel to follow new UX mocks. https://reviewboard.mozilla.org/r/85242/#review84614 > If there isn't enough space, wouldn't you want to have less tiles? We have two options: Either have less tiles or have smaller tiles. In this case we decided to have 4 tiles as minimum and therefore accept to have tiles smaller than the desired tiles if they do not fit. > Couldn't this end up being a negative value, depending on what your desiredTileWidth is in relation to w? This is for the case where we theoretically could fit more than 6 tiles. So there's at least room for one more tile -> After calculating the space for the maximum of 6 tiles there should always be space left that we will use as padding on the left and right (This is for tablets). > placeholder? Yeah, tools:text is just for displaying this text in the UI builder/preview.

Pushed by s.kaspari@gmail.com: https://hg.mozilla.org/integration/autoland/rev/738471dfbee1 Update activity stream panel to follow new UX mocks. r=Grisha