[cr143][Android] Changes for MVT container sizing and centering on Tablets
Chromium changes: https://chromium.googlesource.com/chromium/src/+/a11ac5d97f40076f5d1502084c6ef8972cff2a22 commit a11ac5d97f40076f5d1502084c6ef8972cff2a22 Author: Samuel Huang <huangs@chromium.org> Date: Thu Oct 2 12:00:50 2025 -0700 [MVT] Fix MVT container sizing and centering on Tablets. By "blocks" we include MVT tiles and the "Add now" button. On Tablets, blocks and its container have special behavior: 1. Depending on **initial** block count, these render in modes: * Centered: With "few" blocks present: The container is WRAP_CONTENT, and "edge margins" (outer margin of outer blocks) expanded so blocks are centered and immobile. * Overflowed: With "many" blocks present: The container is MATCH_PARENT, and "edge margins" are assigned a fixed value. The blocks become scrollable. 2. When changing from "many" blocks to "few" blocks, the Overflowed state remains. In this case, blocks no longer scroll, and are start- aligned instead of centered. (2) was intentional, presumably to reduce tiles "jumping" on repeated tile removal. NewTabPageLayout (for container) and TilesLinearLayout (for "edge margins") implemented this by doing the following: * Store initial tile count and block count. * Use these counts and width constants to compute "content width". * If "content width" <= "container" size then Centered (AKA "full filled", "all filled"); else Overflowed. Now, MVT on tablets has some longstanding oddities O1. When Centered, the container might not reach full width. O2. Container may shrink on repeated tile removal (and centered) -- this is a cross between Centered (re. WRAP_CONTENT) and Overflowed (re. fixed "edge margin") modes. O3. When changing from "few" blocks to "many" blocks (by adding Custom Tiles), Overflow mode does kick in sometimes, leading to blocks appearing off-screen but cannot be scrolled to. This CL fixes the oddities and removes behavior (2); now Centered and Overflowed behavior depends on **current** (instead of the initial) block count -- this fixes (O3). High level changes: * Stop storing initial tile and block counts. * Centralize Centered / Overflowed decision, including "content width" computation, to MostVisitedTilesLayout. * Change MVT child addition semantics from {Tile, non-Tile} to {Tile, Divider, UI View (for the "Add new" button)}. Bugs that were found and fixed: * The duplicated Centered / Overflowed decision logic were inconsistent; the NewTabPageLayout version (for WRAP_CONTENT / MATCH_PARENT) didn't account for Custom Tile changes, i.e., divider & "Add new" button. This led to (O2), and is fixed when we dedup the logic. * When "divider gap" was changed to same as "tile gap" in crrev.com/c/6780677 , we didn't update "content width" calculation. This also led to (O2), and is fixed by having dividers not contribute to "content width". * Resources.getDimension() returns (float) PX value, and NOT DP. The misunderstanding led to explicit conversions that made the width of the "Add new" too large (TileRenderer.renderTileSection()). This led to (O1). Fixing this also simplified code. Bug: 447086773, 388782412 Change-Id: Id1400e672f1f0be11b4cc218cd20521988c3455f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7004884 Commit-Queue: Samuel Huang <huangs@chromium.org> Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org> Reviewed-by: Xi Han <hanxi@chromium.org> Cr-Commit-Position: refs/heads/main@{#1524365}
This commit is contained in:
Artem Samoilenko
@@ -46,13 +46,14 @@
|
||||
|
||||
-keep class org.chromium.chrome.browser.ntp.NewTabPageLayout {
|
||||
*** mMvTilesContainerLayout;
|
||||
*** mLogoCoordinator;
|
||||
*** mInitialTileNum;
|
||||
*** mMvtContentFits;
|
||||
*** mProfile;
|
||||
*** initializeSiteSectionView(...);
|
||||
*** setSearchProviderTopMargin(...);
|
||||
*** setSearchProviderBottomMargin(...);
|
||||
*** getLogoMargin(...);
|
||||
*** calculateTabletMvtWidth(...);
|
||||
*** updateMvtOnTablet(...);
|
||||
}
|
||||
|
||||
-keep class org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesMediator {
|
||||
|
||||
@@ -75,7 +75,6 @@ import org.chromium.chrome.browser.feed.FeedSurfaceScrollDelegate;
|
||||
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
|
||||
import org.chromium.chrome.browser.local_database.DatabaseHelper;
|
||||
import org.chromium.chrome.browser.local_database.TopSiteTable;
|
||||
import org.chromium.chrome.browser.logo.LogoCoordinator;
|
||||
import org.chromium.chrome.browser.ntp_background_images.NTPBackgroundImagesBridge;
|
||||
import org.chromium.chrome.browser.ntp_background_images.model.NTPImage;
|
||||
import org.chromium.chrome.browser.ntp_background_images.model.SponsoredTab;
|
||||
@@ -99,6 +98,7 @@ import org.chromium.chrome.browser.settings.BackgroundImagesPreferences;
|
||||
import org.chromium.chrome.browser.settings.BraveNewsPreferencesV2;
|
||||
import org.chromium.chrome.browser.settings.SettingsNavigationFactory;
|
||||
import org.chromium.chrome.browser.suggestions.tile.BraveMostVisitedTilesLayoutBase;
|
||||
import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesLayout;
|
||||
import org.chromium.chrome.browser.suggestions.tile.TileGroup.Delegate;
|
||||
import org.chromium.chrome.browser.tab.Tab;
|
||||
import org.chromium.chrome.browser.tab.TabAttributes;
|
||||
@@ -128,13 +128,12 @@ public class BraveNewTabPageLayout extends NewTabPageLayout
|
||||
private static final int MINIMUM_VISIBLE_HEIGHT_THRESHOLD = 50;
|
||||
private static final int HOUR_MS = 3_600_000;
|
||||
|
||||
// To be removed in bytecode, parent variable will be used instead.
|
||||
// Variables below will be removed in bytecode, variables from parent class will be used
|
||||
// instead.
|
||||
private ViewGroup mMvTilesContainerLayout;
|
||||
|
||||
@SuppressWarnings("UnusedVariable")
|
||||
private LogoCoordinator mLogoCoordinator;
|
||||
|
||||
private Integer mInitialTileNum;
|
||||
private boolean mMvtContentFits;
|
||||
|
||||
// Own members.
|
||||
private WindowAndroid mWindowAndroid;
|
||||
@@ -146,7 +145,6 @@ public class BraveNewTabPageLayout extends NewTabPageLayout
|
||||
// To be removed in bytecode, parent variable will be used instead.
|
||||
private Profile mProfile;
|
||||
private SponsoredTab mSponsoredTab;
|
||||
private boolean mIsTablet;
|
||||
|
||||
private BitmapDrawable mImageDrawable;
|
||||
|
||||
@@ -1205,7 +1203,6 @@ public class BraveNewTabPageLayout extends NewTabPageLayout
|
||||
composeplateUrlSupplier);
|
||||
mNTPBackgroundImagesBridge = NTPBackgroundImagesBridge.getInstance(mProfile);
|
||||
mNTPBackgroundImagesBridge.setNewTabPageListener(mNewTabPageListener);
|
||||
mIsTablet = isTablet;
|
||||
mWindowAndroid = windowAndroid;
|
||||
|
||||
assert mMvTilesContainerLayout != null : "Something has changed in the upstream!";
|
||||
@@ -1578,23 +1575,16 @@ public class BraveNewTabPageLayout extends NewTabPageLayout
|
||||
});
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
|
||||
if (mIsTablet) {
|
||||
if (mInitialTileNum == null) {
|
||||
// In the upstream `mMvTilesContainerLayout` is added as a view in
|
||||
// `insertSiteSectionView`/`initializeSiteSectionView`.
|
||||
// We override `insertSiteSectionView`/`initializeSiteSectionView` to add
|
||||
// `mMvTilesContainerLayout` in our own
|
||||
// RecyclerView to have own NTP UI.
|
||||
// Thus upstream's NewTabPageLayout.findViewById does not see `mv_tiles_layout` and
|
||||
// returns null.
|
||||
mInitialTileNum =
|
||||
((ViewGroup) mMvTilesContainerLayout.findViewById(R.id.mv_tiles_layout))
|
||||
.getChildCount();
|
||||
}
|
||||
}
|
||||
public void calculateTabletMvtWidth(int totalWidth) {
|
||||
if (mMvTilesContainerLayout.getVisibility() == GONE) return;
|
||||
|
||||
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
|
||||
MostVisitedTilesLayout mvTilesLayout =
|
||||
mMvTilesContainerLayout.findViewById(R.id.mv_tiles_layout);
|
||||
mMvtContentFits = mvTilesLayout.contentFitsOnTablet(totalWidth);
|
||||
updateMvtOnTablet();
|
||||
}
|
||||
|
||||
private void updateMvtOnTablet() {
|
||||
assert false : "This method should be removed in the bytecode!";
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user