Bug 1399683: Change padding of content view rather than self in onSizeChanged. r=sebastian
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 21 Sep 2017 15:40:55 -0700
changeset 382414 75c688fefb300e8b2be69e33665f5e2fcf24d559
parent 382413 f62add4838600d2c7405eec95fc5d965161123c7
child 382415 af867bbd41ee73764f5a7adf513ad7ca2d5e50dd
push id32558
push userkwierso@gmail.com
push dateFri, 22 Sep 2017 21:29:46 +0000
treeherdermozilla-central@61e58a7d800b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1399683
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1399683: Change padding of content view rather than self in onSizeChanged. r=sebastian To be honest, I'm not sure why this works. onSizeChanged seems to be a callback to notify the user that your layout has completed and not a place to update layout params. However, it makes *slightly* more sense that you could update your children's layout from this view, which is what this patch changes the code to do. I think I could figure out why this works if I dug further into [1] but I have other things to focus on. I don't think this is the most correct solution, but it is likely the smallest and least risky change we can make to fix this issue, which is ideal because we'd like to uplift this to the 57 Beta. A more correct solution would override the Activity Stream RecyclerView and provide different desired measurements to its parent so that the new size is set *during* layout rather than after it, which would make the layout process more efficient. However, this type of code is less commonly written day-to-day by developers so it's harder to write, harder to maintain, and I'd have to read a lot of [1] in order to write it. :) [1]: https://developer.android.com/guide/topics/ui/how-android-draws.html MozReview-Commit-ID: AceUvDYGWCR
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java
@@ -49,40 +49,41 @@ public class ActivityStreamPanel extends
      * Number of highlights that should be returned (max).
      */
     private static final int HIGHLIGHTS_LIMIT = 10;
 
     public static final String PREF_POCKET_ENABLED = "pref_activitystream_pocket_enabled";
     public static final String PREF_VISITED_ENABLED = "pref_activitystream_visited_enabled";
     public static final String PREF_BOOKMARKS_ENABLED = "pref_activitystream_recentbookmarks_enabled";
 
+    private final RecyclerView contentRecyclerView;
+
     private int desiredTileWidth;
     private int tileMargin;
     private final SharedPreferences sharedPreferences;
 
     public ActivityStreamPanel(Context context, AttributeSet attrs) {
         super(context, attrs);
 
         setBackgroundColor(ContextCompat.getColor(context, R.color.photon_browser_toolbar_bg));
 
         inflate(context, R.layout.as_content, this);
 
         adapter = new StreamRecyclerAdapter();
         sharedPreferences = GeckoSharedPrefs.forProfile(context);
 
-        final RecyclerView rv = (RecyclerView) findViewById(R.id.activity_stream_main_recyclerview);
+        contentRecyclerView = (RecyclerView) findViewById(R.id.activity_stream_main_recyclerview);
+        contentRecyclerView.setAdapter(adapter);
+        contentRecyclerView.setLayoutManager(new LinearLayoutManager(getContext()));
+        contentRecyclerView.setHasFixedSize(true);
+        // Override item animations to avoid horrible topsites refreshing
+        contentRecyclerView.setItemAnimator(new StreamItemAnimator());
+        contentRecyclerView.addItemDecoration(new HighlightsDividerItemDecoration(context));
 
-        rv.setAdapter(adapter);
-        rv.setLayoutManager(new LinearLayoutManager(getContext()));
-        rv.setHasFixedSize(true);
-        // Override item animations to avoid horrible topsites refreshing
-        rv.setItemAnimator(new StreamItemAnimator());
-        rv.addItemDecoration(new HighlightsDividerItemDecoration(context));
-
-        RecyclerViewClickSupport.addTo(rv)
+        RecyclerViewClickSupport.addTo(contentRecyclerView)
                 .setOnItemClickListener(adapter)
                 .setOnItemLongClickListener(adapter);
 
         final Resources resources = getResources();
         desiredTileWidth = resources.getDimensionPixelSize(R.dimen.activity_stream_desired_tile_width);
         tileMargin = resources.getDimensionPixelSize(R.dimen.activity_stream_base_margin);
 
         ActivityStreamTelemetry.Extras.setGlobal(
@@ -145,27 +146,27 @@ public class ActivityStreamPanel extends
 
         // Calculate how many tiles fit into the available horizontal space if we are using our
         // desired tile width.
         int fittingTiles = (w - tileMargin) / (desiredTileWidth + tileMargin);
 
         if (fittingTiles <= TopSitesPage.NUM_COLUMNS) {
             // We can't fit all tiles (or they fit exactly) if we are using the desired tiles width.
             // We will still show all tiles but they might be smaller than what we would like them to be.
-            setPadding(0, 0, 0, 0);
+            contentRecyclerView.setPadding(0, 0, 0, 0);
         } else if (fittingTiles > TopSitesPage.NUM_COLUMNS) {
             // We can fit more tiles than we want to display. Calculate how much space we need and
             // use the remaining space as padding on the left and right.
             int needed = TopSitesPage.NUM_COLUMNS * (desiredTileWidth + tileMargin) + tileMargin;
             int padding = (w - needed) / 2;
 
             // With the padding applied we have less space available for the tiles
             w = needed;
 
-            setPadding(padding, 0, padding, 0);
+            contentRecyclerView.setPadding(padding, 0, padding, 0);
         }
 
         // Now calculate how large an individual tile is
         final int tilesSize = (w - (TopSitesPage.NUM_COLUMNS * tileMargin) - tileMargin) / TopSitesPage.NUM_COLUMNS;
 
         adapter.setTileSize(tilesSize);
     }