From 0896159f33c40980e46c43ff7c56c84dcf7b4aa1 Mon Sep 17 00:00:00 2001
From: Kyle Milka <kmilka@chromium.org>
Date: Thu, 6 Dec 2018 19:00:38 +0000
Subject: [PATCH] [NTP] Remove UI MD flag
Remove flag for ntp md ui. Change the new styling to be the
default and remove the md class, this mainly affects the fakebox
as the icons are styled via md-icons.
Mostly a reland of https://crrev.com/c/1265024.
Bug: 905386
Change-Id: I62db12fe3cea1e85ed615f1ad2fbb9ea211f585e
Reviewed-on: https://chromium-review.googlesource.com/c/1361720
Reviewed-by: Kristi Park <kristipark@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Kyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614432}
---
chrome/browser/about_flags.cc | 4 -
chrome/browser/flag-metadata.json | 5 --
chrome/browser/flag_descriptions.cc | 4 -
chrome/browser/flag_descriptions.h | 3 -
.../browser/resources/local_ntp/local_ntp.css | 85 ++++---------------
.../browser/resources/local_ntp/local_ntp.js | 13 ---
chrome/browser/search/local_ntp_source.cc | 5 +-
chrome/browser/search/ntp_features.cc | 17 +---
chrome/browser/search/ntp_features.h | 4 -
.../ui/search/local_ntp_browsertest.cc | 2 +-
.../data/local_ntp/local_ntp_browsertest.js | 26 ------
.../variations/fieldtrial_testing_config.json | 3 +-
12 files changed, 19 insertions(+), 152 deletions(-)
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index b27ac59049504..dc362c4647863 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -3972,10 +3972,6 @@ const FeatureEntry kFeatureEntries[] = {
{"ntp-icons", flag_descriptions::kNtpIconsName,
flag_descriptions::kNtpIconsDescription, kOsDesktop,
FEATURE_VALUE_TYPE(features::kNtpIcons)},
-
- {"ntp-ui-md", flag_descriptions::kNtpUIMdName,
- flag_descriptions::kNtpUIMdDescription, kOsDesktop,
- FEATURE_VALUE_TYPE(features::kNtpUIMd)},
#endif // OS_WIN || OS_MACOSX || OS_LINUX
#if defined(OS_ANDROID)
diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
index b58efe6054899..cf74dd386c233 100644
--- a/chrome/browser/flag-metadata.json
+++ b/chrome/browser/flag-metadata.json
@@ -2524,11 +2524,6 @@
"owners": [ "kristipark", "ramyan" ],
"expiry_milestone": 73
},
- {
- "name": "ntp-ui-md",
- "owners": [ "kmilka", "ramyan" ],
- "expiry_milestone": 73
- },
{
"name": "num-raster-threads",
// "owners": [ "your-team" ],
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
index 68bc4dab1aac8..c2c0f9b734201 100644
--- a/chrome/browser/flag_descriptions.cc
+++ b/chrome/browser/flag_descriptions.cc
@@ -1318,10 +1318,6 @@ const char kNtpIconsDescription[] =
"Show Material Design icons on the New Tab Page, instead of Most Visited "
"tiles. Implicitly enables 'ntp-ui-md'.";
-const char kNtpUIMdName[] = "New Tab Page Material Design UI";
-const char kNtpUIMdDescription[] =
- "Updates the New Tab Page with Material Design elements.";
-
const char kNumRasterThreadsName[] = "Number of raster threads";
const char kNumRasterThreadsDescription[] =
"Specify the number of raster threads.";
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
index 28ddd9bfb3b00..30479d85482a1 100644
--- a/chrome/browser/flag_descriptions.h
+++ b/chrome/browser/flag_descriptions.h
@@ -807,9 +807,6 @@ extern const char kNtpCustomLinksDescription[];
extern const char kNtpIconsName[];
extern const char kNtpIconsDescription[];
-extern const char kNtpUIMdName[];
-extern const char kNtpUIMdDescription[];
-
extern const char kNumRasterThreadsName[];
extern const char kNumRasterThreadsDescription[];
extern const char kNumRasterThreadsOne[];
diff --git a/chrome/browser/resources/local_ntp/local_ntp.css b/chrome/browser/resources/local_ntp/local_ntp.css
index 0ebaae016eeab..448a9f71c6801 100644
--- a/chrome/browser/resources/local_ntp/local_ntp.css
+++ b/chrome/browser/resources/local_ntp/local_ntp.css
@@ -292,32 +292,21 @@ body.alternate-logo #logo-non-white {
}
#fakebox {
- background-color: #fff;
- border-radius: 2px;
- box-shadow: 0 2px 2px 0 rgba(0, 0, 0, 0.16), 0 0 0 1px rgba(0, 0, 0, 0.08);
+ background-color: rgb(241, 243, 244);
+ border-radius: 22px;
cursor: text;
font-size: 18px;
height: 44px;
line-height: 36px;
- margin: 0 calc(var(--tile-margin) / 2 + 1px) 0 calc(var(--tile-margin) / 2);
- outline: none;
- position: relative;
- transition: box-shadow 200ms cubic-bezier(0.4, 0, 0.2, 1);
-}
-
-.md #fakebox {
- background-color: rgb(241, 243, 244);
- border-radius: 22px;
- box-shadow: none;
margin: 0 auto;
max-width: 560px;
opacity: 1;
- transition: background-color 300ms ease-in-out;
+ position: relative;
/* Transition should be similar to .mv-tile/.md-tile opacity transition. */
- transition: opacity 200ms;
+ transition: background-color 300ms ease-in-out, opacity 200ms
}
-.md #fakebox:hover {
+#fakebox:hover {
background-color: rgb(232, 234, 237);
}
@@ -325,50 +314,33 @@ body.alternate-logo #logo-non-white {
display: none;
}
-#fakebox:hover,
-body.fakebox-focused #fakebox {
- box-shadow: 0 3px 8px 0 rgba(0, 0, 0, 0.2), 0 0 0 1px rgba(0, 0, 0, 0.08);
-}
-
-.md #fakebox:hover,
-body.md.fakebox-focused #fakebox {
- box-shadow: none;
-}
-
#fakebox > input {
bottom: 0;
box-sizing: border-box;
left: 0;
margin: 0;
opacity: 0;
- padding-left: 8px;
+ padding-left: 20px;
position: absolute;
top: 0;
width: 100%;
}
-.md #fakebox > input {
- padding-left: 20px;
-}
-
html[dir=rtl] #fakebox > input {
padding-left: 0;
- padding-right: 8px;
- right: 0;
-}
-
-html[dir=rtl] .md #fakebox > input {
padding-right: 20px;
+ right: 0;
}
#fakebox-text {
bottom: 4px;
- color: rgba(0, 0, 0, 0.42);
- font-family: arial, sans-serif;
- font-size: 16px;
- left: 13px;
+ color: rgb(128, 134, 139);
+ font-family: 'Roboto', arial, sans-serif;
+ font-size: 14px;
+ left: 0;
margin-top: 1px;
overflow: hidden;
+ padding-left: 20px;
position: absolute;
right: 13px;
text-align: initial;
@@ -379,20 +351,8 @@ html[dir=rtl] .md #fakebox > input {
white-space: nowrap;
}
-.md #fakebox-text {
- color: rgb(128, 134, 139);
- font-family: 'Roboto', arial, sans-serif;
- font-size: 14px;
- left: 0;
- padding-left: 20px;
-}
-
html[dir=rtl] #fakebox-text {
left: auto;
- right: 13px;
-}
-
-html[dir=rtl] .md #fakebox-text {
padding-right: 20px;
right: 0;
}
@@ -400,23 +360,15 @@ html[dir=rtl] .md #fakebox-text {
#fakebox-cursor {
background: #333;
bottom: 12px;
- left: 13px;
+ left: 20px;
position: absolute;
top: 12px;
visibility: hidden;
width: 1px;
}
-.md #fakebox-cursor {
- left: 20px;
-}
-
html[dir=rtl] #fakebox-cursor {
left: auto;
- right: 13px;
-}
-
-html[dir=rtl] .md #fakebox-cursor {
right: 20px;
}
@@ -425,25 +377,18 @@ html[dir=rtl] .md #fakebox-cursor {
background-size: 24px 24px;
bottom: 0;
cursor: pointer;
+ margin-right: 12px;
padding: 22px 12px 0;
position: absolute;
right: 0;
top: 0;
- width: 41px;
-}
-
-.md #fakebox-microphone {
- margin-right: 12px;
width: 28px;
}
html[dir=rtl] #fakebox-microphone {
left: 0;
- right: auto;
-}
-
-html[dir=rtl] .md #fakebox-microphone {
margin-left: 12px;
+ right: auto;
}
@keyframes blink {
diff --git a/chrome/browser/resources/local_ntp/local_ntp.js b/chrome/browser/resources/local_ntp/local_ntp.js
index 03c5e5f0ce8ac..38c2b58e9be72 100644
--- a/chrome/browser/resources/local_ntp/local_ntp.js
+++ b/chrome/browser/resources/local_ntp/local_ntp.js
@@ -95,7 +95,6 @@ var CLASSES = {
HIDE_NOTIFICATION: 'notice-hide',
INITED: 'inited', // Reveals the <body> once init() is done.
LEFT_ALIGN_ATTRIBUTION: 'left-align-attribution',
- MATERIAL_DESIGN: 'md', // Applies Material Design styles to the page
MATERIAL_DESIGN_ICONS:
'md-icons', // Applies Material Design styles to Most Visited.
// Vertically centers the most visited section for a non-Google provided page.
@@ -923,19 +922,9 @@ function handlePostMessage(event) {
function enableMDIcons() {
$(IDS.MOST_VISITED).classList.add(CLASSES.MATERIAL_DESIGN_ICONS);
$(IDS.TILES).classList.add(CLASSES.MATERIAL_DESIGN_ICONS);
- enableMD();
animations.addRippleAnimations();
}
-
-/**
- * Enables Material Design styles for all NTP components except Most Visited.
- */
-function enableMD() {
- document.body.classList.add(CLASSES.MATERIAL_DESIGN);
-}
-
-
/**
* Prepares the New Tab Page by adding listeners, the most visited pages
* section, and Google-specific elements for a Google-provided page.
@@ -990,8 +979,6 @@ function init() {
if (configData.isGooglePage) {
if (configData.isMDIconsEnabled || configData.isCustomLinksEnabled) {
enableMDIcons();
- } else if (configData.isMDUIEnabled) {
- enableMD();
}
if (configData.isCustomLinksEnabled) {
diff --git a/chrome/browser/search/local_ntp_source.cc b/chrome/browser/search/local_ntp_source.cc
index 8d673afa18dca..5bfd2747eb12c 100644
--- a/chrome/browser/search/local_ntp_source.cc
+++ b/chrome/browser/search/local_ntp_source.cc
@@ -162,8 +162,7 @@ std::unique_ptr<base::DictionaryValue> GetTranslatedStrings(bool is_google) {
if (is_google) {
AddString(translated_strings.get(), "searchboxPlaceholder",
- features::IsMDUIEnabled() ? IDS_GOOGLE_SEARCH_BOX_EMPTY_HINT_MD
- : IDS_GOOGLE_SEARCH_BOX_EMPTY_HINT);
+ IDS_GOOGLE_SEARCH_BOX_EMPTY_HINT_MD);
// Custom Backgrounds
AddString(translated_strings.get(), "customizeBackground",
@@ -515,8 +514,6 @@ class LocalNtpSource::SearchConfigurationProvider
content::BrowserAccessibilityState::GetInstance()
->IsAccessibleBrowser());
- config_data.SetBoolean("isMDUIEnabled", features::IsMDUIEnabled());
-
config_data.SetBoolean("isMDIconsEnabled", features::IsMDIconsEnabled());
if (is_google) {
diff --git a/chrome/browser/search/ntp_features.cc b/chrome/browser/search/ntp_features.cc
index 08c670801c676..4d8c98c17378b 100644
--- a/chrome/browser/search/ntp_features.cc
+++ b/chrome/browser/search/ntp_features.cc
@@ -20,15 +20,10 @@ const base::Feature kNtpBackgrounds{"NewTabPageBackgrounds",
base::FEATURE_ENABLED_BY_DEFAULT};
// If enabled, the user will see the Most Visited tiles updated with Material
-// Design elements. Implicitly enables |kNtpUIMd|.
+// Design elements.
const base::Feature kNtpIcons{"NewTabPageIcons",
base::FEATURE_ENABLED_BY_DEFAULT};
-// If enabled, the user will see the New Tab Page updated with Material Design
-// elements.
-const base::Feature kNtpUIMd{"NewTabPageUIMd",
- base::FEATURE_ENABLED_BY_DEFAULT};
-
// If enabled, the user will sometimes see promos on the NTP.
const base::Feature kPromosOnLocalNtp{"PromosOnLocalNtp",
base::FEATURE_DISABLED_BY_DEFAULT};
@@ -57,14 +52,4 @@ bool IsMDIconsEnabled() {
base::FeatureList::IsEnabled(features::kExperimentalUi);
}
-bool IsMDUIEnabled() {
- return base::FeatureList::IsEnabled(kNtpUIMd) ||
- // MD UI changes are implicitly enabled if Material Design icons,
- // custom link, or custom backgrounds are enabled.
- base::FeatureList::IsEnabled(kNtpIcons) ||
- base::FeatureList::IsEnabled(kNtpBackgrounds) ||
- base::FeatureList::IsEnabled(ntp_tiles::kNtpCustomLinks) ||
- base::FeatureList::IsEnabled(features::kExperimentalUi);
-}
-
} // namespace features
diff --git a/chrome/browser/search/ntp_features.h b/chrome/browser/search/ntp_features.h
index e9b36ae62be57..b397c43b13670 100644
--- a/chrome/browser/search/ntp_features.h
+++ b/chrome/browser/search/ntp_features.h
@@ -15,7 +15,6 @@ namespace features {
extern const base::Feature kDoodlesOnLocalNtp;
extern const base::Feature kNtpBackgrounds;
extern const base::Feature kNtpIcons;
-extern const base::Feature kNtpUIMd;
extern const base::Feature kPromosOnLocalNtp;
extern const base::Feature kSearchSuggestionsOnLocalNtp;
extern const base::Feature kUseGoogleLocalNtp;
@@ -29,9 +28,6 @@ bool IsCustomBackgroundsEnabled();
// Returns whether the Material Design UI for Most Visited is enabled.
bool IsMDIconsEnabled();
-// Returns whether the Material Design UI is enabled on the New Tab Page.
-bool IsMDUIEnabled();
-
} // namespace features
#endif // CHROME_BROWSER_SEARCH_NTP_FEATURES_H_
diff --git a/chrome/browser/ui/search/local_ntp_browsertest.cc b/chrome/browser/ui/search/local_ntp_browsertest.cc
index faaa1f17713a8..cbaef19a2e9f5 100644
--- a/chrome/browser/ui/search/local_ntp_browsertest.cc
+++ b/chrome/browser/ui/search/local_ntp_browsertest.cc
@@ -595,7 +595,7 @@ class LocalNTPCustomLinksTest : public LocalNTPTest {
LocalNTPCustomLinksTest()
: LocalNTPTest(
/*enabled_features=*/{features::kUseGoogleLocalNtp,
- features::kNtpUIMd, features::kNtpIcons,
+ features::kNtpIcons,
ntp_tiles::kNtpCustomLinks},
/*disabled_features=*/{}) {}
diff --git a/chrome/test/data/local_ntp/local_ntp_browsertest.js b/chrome/test/data/local_ntp/local_ntp_browsertest.js
index 76b21114ed8f6..a9e196d376f30 100644
--- a/chrome/test/data/local_ntp/local_ntp_browsertest.js
+++ b/chrome/test/data/local_ntp/local_ntp_browsertest.js
@@ -73,32 +73,6 @@ test.localNtp.testMostVisitedContents = function() {
window.chrome.embeddedSearch.newTabPage.mostVisited[0].rid));
};
-/**
- * Tests that the GM2 style is applied when the flag is enabled.
- */
-test.localNtp.testMDApplied = function() {
- // Turn off voice search to avoid reinitializing the speech object
- configData.isVoiceSearchEnabled = false;
-
- configData.isMDUIEnabled = true;
- initLocalNTP(/*isGooglePage=*/true);
- assertTrue(document.body.classList.contains('md'));
-}
-
-/**
- * Tests that the GM2 style is not applied when the flag is disabled.
- */
-test.localNtp.testMDNotApplied = function() {
- // Turn off voice search to avoid reinitializing the speech object
- configData.isVoiceSearchEnabled = false;
-
- configData.isMDUIEnabled = false;
- configData.isMDIconsEnabled = false;
- configData.isCustomLinksEnabled = false;
- initLocalNTP(/*isGooglePage=*/true);
- assertFalse(document.body.classList.contains('md'));
-}
-
// ****************************** ADVANCED TESTS ******************************
// Advanced tests are controlled from the native side. The helpers here are
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
index 007389b392e16..7d64e353f9db4 100644
--- a/testing/variations/fieldtrial_testing_config.json
+++ b/testing/variations/fieldtrial_testing_config.json
@@ -2751,8 +2751,7 @@
"enable_features": [
"NewTabPageBackgrounds",
"NewTabPageCustomLinks",
- "NewTabPageIcons",
- "NewTabPageUIMd"
+ "NewTabPageIcons"
]
}
]