Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(579)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2167973004: Revert of 📰 Adjust the card display depending on the screen width. (Closed)

Created:
4 years, 5 months ago by Dan Beam
Modified:
4 years, 5 months ago
CC:
chromium-reviews, ntp-dev+reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@ZineTabletUI
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of 📰 Adjust the card display depending on the screen width. (patchset #8 id:140001 of https://codereview.chromium.org/2149333003/ ) Reason for revert: Broke Android compile: util.build_utils.CalledProcessError: Command failed: ( cd /mnt/data/b/c/b/Android_Arm64_Builder__dbg_/src/out/Debug; javac -g -encoding UTF-8 -classpath lib.java/chrome/android/chrome_java.interface.jar:lib.java/base/base_java.interface.jar:lib.java/base/base_java_test_support.interface.jar:lib.java/base/base_junit_test_support.interface.jar:lib.java/components/bookmarks/common/android/bookmarks_java.interface.jar:lib.java/components/invalidation/impl/java.interface.jar:lib.java/components/web_restrictions/web_restrictions_java.interface.jar:lib.java/content/public/android/content_java.interface.jar:lib.java/net/android/net_java.interface.jar:lib.java/sync/sync_java_test_support.interface.jar:lib.java/sync/android/sync_java.interface.jar:lib.java/third_party/WebKit/public/blink_headers_java.interface.jar:lib.java/third_party/android_tools/android_support_v7_mediarouter_java__jar_1.interface.jar:lib.java/third_party/android_tools/android_support_v7_mediarouter_java__jar_2.interface.jar:lib.java/third_party/android_tools/android_support_v7_recyclerview_java__jar_1.interface.jar:lib.java/third_party/cacheinvalidation/cacheinvalidation_javalib.interface.jar:lib.java/third_party/junit/hamcrest.interface.jar:lib.java/ui/android/ui_java.interface.jar:lib.java/third_party/android_tools/google_play_services_default_java.interface.jar:lib.java/testing/android/junit/junit_test_support.interface.jar:lib.java/third_party/junit/junit.interface.jar:lib.java/third_party/mockito/mockito_java.interface.jar:lib.java/third_party/robolectric/android-all-4.3_r2-robolectric-0.interface.jar:lib.java/third_party/robolectric/robolectric_java.interface.jar -sourcepath '' -Xlint:unchecked -Xlint:deprecation -d /tmp/tmphznxR6/classes ../../chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackupAgentTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/ChromeBackgroundServiceWaiterTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/SSLClientCertificateRequestTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionControllerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/cookies/CanonicalCookieTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/crash/LogcatExtractionCallableTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtilsTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/gcore/GoogleApiClientHelperTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/invalidation/InvalidationControllerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/remote/AbstractMediaRouteControllerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/remote/RemoteVideoInfoTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterRouteTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterSinkObservationTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/ChromeMediaRouterTestBase.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/CastMessageHandlerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/DiscoveryCallbackTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/JSONTestUtils.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/MediaSourceTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/media/router/cast/TestUtils.java ../../chrome/android/junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/ntp/NativePageFactoryTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/ntp/TitleUtilTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTaskTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/ClientIdTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserverTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtilsTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/ShadowGcmNetworkManager.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/StubBackgroundSchedulerProcessor.java ../../chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/TaskExtrasPackerTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/omaha/ResponseParserTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/omaha/VersionNumberTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillContactTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/payments/CurrencyStringFormatterTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/snackbar/SnackbarCollectionUnitTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderUnitTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/util/NonThreadSafeTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappDataStorageTest.java ../../chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebappRegistryTest.java /tmp/tmphznxR6/java/android/support/v7/recyclerview/R.java /tmp/tmphznxR6/java/android/support/v7/appcompat/R.java /tmp/tmphznxR6/java/android/support/v7/mediarouter/R.java /tmp/tmphznxR6/java/android/support/design/R.java /tmp/tmphznxR6/java/com/google/android/gms/R.java /tmp/tmphznxR6/java/org/chromium/ui/R.java /tmp/tmphznxR6/java/org/chromium/content/R.java /tmp/tmphznxR6/java/org/chromium/components/web_contents_delegate_android/R.java /tmp/tmphznxR6/java/org/chromium/chrome/R.java /tmp/tmphznxR6/java/org/chromium/third_party/android/R.java /tmp/tmphznxR6/java/org/chromium/third_party/android/media/R.java ) ../../chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:162: error: constructor NewTabPageAdapter in class NewTabPageAdapter cannot be applied to given types; NewTabPageAdapter ntpa = new NewTabPageAdapter(mNewTabPageManager, null, mSnippetsBridge); ^ required: NewTabPageManager,NewTabPageLayout,SnippetsBridge,UiConfig found: NewTabPageManager,<null>,SnippetsBridge reason: actual and formal argument lists differ in length https://build.chromium.org/p/chromium.linux/builders/Android%20Builder/builds/70053/steps/compile/logs/stdio Original issue's description: > [NTP Snippets] Adjust the card display depending on the screen width. > > Changes the lines to go from always 2 to at most 2 by default, so that > we don't show empty lines on very large screens. > For smaller screens, the title can go up to 4 lines, and we then hide > the description. > On bigger screens, we add space on the side of the cards > > Measures used: > < 360dp: Narrow -> 4 lines title > >= 360dp: Regular -> 2 + 2 lines > >= 600dp: Wide -> 2 + 2 lines, 48dp gutters around the cards > > Preview: https://goo.gl/photos/prJ42tvP4jzwiCn3A > BUG=625628, 624333 > > Committed: https://crrev.com/7c430bb62b6269b9f1bce082083e00324a26daee > Cr-Commit-Position: refs/heads/master@{#406923} TBR=bauerb@chromium.org,mvanouwerkerk@chromium.org,peconn@chromium.org,dgn@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=625628, 624333 Committed: https://crrev.com/91c4694ecfdd34810a9f267698c6f236ffefeac5 Cr-Commit-Position: refs/heads/master@{#406932}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -350 lines) Patch
M chrome/android/java/res/layout/new_tab_page_snippets_card.xml View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/android/java/res/layout/new_tab_page_snippets_header.xml View 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/res/values/dimens.xml View 1 chunk +0 lines, -1 line 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/ntp/DisplayStyleObserver.java View 1 chunk +0 lines, -16 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java View 5 chunks +1 line, -17 lines 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/ntp/UiConfig.java View 1 chunk +0 lines, -113 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java View 6 chunks +4 lines, -29 lines 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/DisplayStyleObserverAdapter.java View 1 chunk +0 lines, -82 lines 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/MarginResizer.java View 1 chunk +0 lines, -41 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java View 6 chunks +5 lines, -9 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java View 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java View 3 chunks +2 lines, -19 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetHeaderViewHolder.java View 2 chunks +11 lines, -8 lines 0 comments Download
M chrome/android/java_sources.gni View 3 chunks +0 lines, -4 lines 0 comments Download
M chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java View 3 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Dan Beam
Created Revert of 📰 Adjust the card display depending on the screen width.
4 years, 5 months ago (2016-07-21 20:14:55 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2167973004/1
4 years, 5 months ago (2016-07-21 20:15:27 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-07-21 20:16:40 UTC) #5
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/91c4694ecfdd34810a9f267698c6f236ffefeac5 Cr-Commit-Position: refs/heads/master@{#406932}
4 years, 5 months ago (2016-07-21 20:22:19 UTC) #7
dgn
4 years, 5 months ago (2016-07-22 08:54:56 UTC) #8
Message was sent while issue was closed. 
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2173723002/ by dgn@chromium.org.

The reason for reverting is: Reland.

Powered by Google App Engine
This is Rietveld 408576698