From 1d95a8f0e313c9afbfd56fa0c007a16ad3881ba9 Mon Sep 17 00:00:00 2001 From: "aa@chromium.org" Date: Tue, 14 Sep 2010 17:56:55 +0000 Subject: Updates to NTP per UI review: 1) Turn off logo/attribution completely when it overlaps with the content. 2) Replace the single-item dropdown menu in the most visited section with a text link to restore blacklisted URLs. The link should only be visible if there are any blacklisted URLs. BUG=55196,55208 TEST=unit_tests --gtest-filter=TopSitesTest.* Review URL: http://codereview.chromium.org/3361015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59397 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/dom_ui/most_visited_handler.cc | 6 ++++- chrome/browser/history/top_sites.cc | 5 ++++ chrome/browser/history/top_sites.h | 3 +++ chrome/browser/history/top_sites_unittest.cc | 5 ++++ chrome/browser/resources/new_new_tab.css | 35 +-------------------------- chrome/browser/resources/new_new_tab.html | 9 ++----- chrome/browser/resources/new_new_tab.js | 7 +++++- chrome/browser/resources/new_tab_theme.css | 8 +++--- chrome/browser/resources/ntp/most_visited.css | 24 ++++++++++++++++++ chrome/browser/resources/ntp/most_visited.js | 13 +++++++++- 10 files changed, 67 insertions(+), 48 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/dom_ui/most_visited_handler.cc b/chrome/browser/dom_ui/most_visited_handler.cc index f152dfe..3542198 100644 --- a/chrome/browser/dom_ui/most_visited_handler.cc +++ b/chrome/browser/dom_ui/most_visited_handler.cc @@ -132,9 +132,13 @@ void MostVisitedHandler::HandleGetMostVisited(const ListValue* args) { void MostVisitedHandler::SendPagesValue() { if (pages_value_.get()) { + history::TopSites* ts = dom_ui_->GetProfile()->GetTopSites(); FundamentalValue first_run(IsFirstRun()); + FundamentalValue has_blacklisted_urls(ts->HasBlacklistedItems()); dom_ui_->CallJavascriptFunction(L"mostVisitedPages", - *(pages_value_.get()), first_run); + *(pages_value_.get()), + first_run, + has_blacklisted_urls); pages_value_.reset(); } } diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc index 2825fff..d746e43 100644 --- a/chrome/browser/history/top_sites.cc +++ b/chrome/browser/history/top_sites.cc @@ -669,6 +669,11 @@ void TopSites::StartMigration() { MigratePinnedURLs(); } +bool TopSites::HasBlacklistedItems() const { + AutoLock lock(lock_); + return !blacklist_->empty(); +} + void TopSites::AddBlacklistedURL(const GURL& url) { AutoLock lock(lock_); RemovePinnedURLLocked(url); diff --git a/chrome/browser/history/top_sites.h b/chrome/browser/history/top_sites.h index 81884d7..1b57a61 100644 --- a/chrome/browser/history/top_sites.h +++ b/chrome/browser/history/top_sites.h @@ -102,6 +102,9 @@ class TopSites : // Blacklisted URLs + // Returns true if there is at least one item in the blacklist. + bool HasBlacklistedItems() const; + // Add a URL to the blacklist. void AddBlacklistedURL(const GURL& url); diff --git a/chrome/browser/history/top_sites_unittest.cc b/chrome/browser/history/top_sites_unittest.cc index 360cd0d..c327647 100644 --- a/chrome/browser/history/top_sites_unittest.cc +++ b/chrome/browser/history/top_sites_unittest.cc @@ -1140,8 +1140,10 @@ TEST_F(TopSitesTest, Blacklisting) { EXPECT_EQ("http://google.com/", urls()[1].url.spec()); EXPECT_EQ(welcome_url(), urls()[2].url); EXPECT_EQ(themes_url(), urls()[3].url); + EXPECT_FALSE(top_sites().HasBlacklistedItems()); top_sites().AddBlacklistedURL(GURL("http://google.com/")); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); { Lock& l = lock(); AutoLock lock(l); // IsBlacklisted must be in a lock. @@ -1162,6 +1164,7 @@ TEST_F(TopSitesTest, Blacklisting) { EXPECT_EQ(themes_url(), urls()[2].url); top_sites().AddBlacklistedURL(welcome_url()); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); top_sites().GetMostVisitedURLs( &c, NewCallback(static_cast(this), @@ -1171,6 +1174,7 @@ TEST_F(TopSitesTest, Blacklisting) { EXPECT_EQ(themes_url(), urls()[1].url); top_sites().RemoveBlacklistedURL(GURL("http://google.com/")); + EXPECT_TRUE(top_sites().HasBlacklistedItems()); { Lock& l = lock(); AutoLock lock(l); // IsBlacklisted must be in a lock. @@ -1187,6 +1191,7 @@ TEST_F(TopSitesTest, Blacklisting) { EXPECT_EQ(themes_url(), urls()[2].url); top_sites().ClearBlacklistedURLs(); + EXPECT_FALSE(top_sites().HasBlacklistedItems()); top_sites().GetMostVisitedURLs( &c, NewCallback(static_cast(this), diff --git a/chrome/browser/resources/new_new_tab.css b/chrome/browser/resources/new_new_tab.css index 62232ce..1e4c921 100644 --- a/chrome/browser/resources/new_new_tab.css +++ b/chrome/browser/resources/new_new_tab.css @@ -232,7 +232,7 @@ html[dir=rtl] #attribution { } #attribution.obscured { - opacity: 0.3; + visibility: hidden; } html[hasattribution=false] #attribution > div { @@ -311,39 +311,6 @@ html[anim=true] .section > h2 > .disclosure { z-index: 2; } -.section > h2 .settings-wrapper { - position: absolute; - top: -1px; - right: 0; - width: 21px; - height: 21px; - z-index: 3; -} - -html[dir=rtl] .section > h2 .settings-wrapper { - left: 0; - right: auto; -} - -.section > h2 .settings { - /* TODO(aa): Need a better image. This one is semi-transparent. Also, I think - a gear would be better. */ - background-color: transparent; - background-image: url(chrome://theme/IDR_BALLOON_WRENCH); - background-position: center center; - background-repeat: no-repeat; - border: 0; - height: 13px; - left: 5px; - position: absolute; - top: 5px; - width: 13px; -} - -.section.hidden > h2 .settings-wrapper { - display: none; -} - .maxiview { padding: 5px 0 30px; position: absolute; diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html index 1a88c6d..1443c7c 100644 --- a/chrome/browser/resources/new_new_tab.html +++ b/chrome/browser/resources/new_new_tab.html @@ -147,9 +147,8 @@ if ('mode' in hashParams) {
-
- -
+
@@ -181,10 +180,6 @@ if ('mode' in hashParams) { - - - -
diff --git a/chrome/browser/resources/new_new_tab.js b/chrome/browser/resources/new_new_tab.js index 7ba47c1..1781c9a 100644 --- a/chrome/browser/resources/new_new_tab.js +++ b/chrome/browser/resources/new_new_tab.js @@ -669,6 +669,10 @@ $('main').addEventListener('click', function(e) { toggleSectionVisibilityAndAnimate(p.getAttribute('section')); }); +$('most-visited-settings').addEventListener('click', function() { + $('clear-all-blacklisted').execute(); +}); + function toggleSectionVisibilityAndAnimate(section) { if (!section) return; @@ -924,9 +928,10 @@ var mostVisited = new MostVisited( useSmallGrid(), shownSections & Section.THUMB); -function mostVisitedPages(data, firstRun) { +function mostVisitedPages(data, firstRun, hasBlacklistedUrls) { logEvent('received most visited pages'); + mostVisited.updateSettingsLink(hasBlacklistedUrls); mostVisited.data = data; mostVisited.layout(); layoutSections(); diff --git a/chrome/browser/resources/new_tab_theme.css b/chrome/browser/resources/new_tab_theme.css index e21eaef..440347e 100644 --- a/chrome/browser/resources/new_tab_theme.css +++ b/chrome/browser/resources/new_tab_theme.css @@ -1,6 +1,6 @@ html, -.section > h2 .settings-wrapper, -.section > h2 span { +.section > h2 span, +#most-visited-settings { background-attachment: fixed; background-color: $2; /* COLOR_NTP_BACKGROUND */ background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?$1); @@ -9,8 +9,8 @@ html, } html[bookmarkbarattached='true'], -html[bookmarkbarattached='true'] .section > h2 .settings-wrapper, -html[bookmarkbarattached='true'] .section > h2 span { +html[bookmarkbarattached='true'] .section > h2 span, +html[bookmarkbarattached='true'] #most-visited-settings { background-position: $4; } diff --git a/chrome/browser/resources/ntp/most_visited.css b/chrome/browser/resources/ntp/most_visited.css index 3512d59..27f2958 100644 --- a/chrome/browser/resources/ntp/most_visited.css +++ b/chrome/browser/resources/ntp/most_visited.css @@ -256,3 +256,27 @@ html[dir=rtl] .thumbnail-container > .title > div { background-size: 150px 93px; } } + +#most-visited-settings { + position: absolute; + top: 1px; + right: 0; + border: 0; + cursor: pointer; + font-size: 70%; + margin: 0; + padding: 0; + text-decoration: underline; + visibility: hidden; + -webkit-padding-start: 3px; + z-index: 3; +} + +html[dir=rtl] #most-visited-settings { + left: 0; + right: auto; +} + +#most-visited:not(.hidden) #most-visited-settings.has-blacklist { + visibility: visible; +} diff --git a/chrome/browser/resources/ntp/most_visited.js b/chrome/browser/resources/ntp/most_visited.js index 5b024b2..c5f1967 100644 --- a/chrome/browser/resources/ntp/most_visited.js +++ b/chrome/browser/resources/ntp/most_visited.js @@ -104,6 +104,13 @@ var MostVisited = (function() { this.data[sourceIndex] = destinationData; }, + updateSettingsLink: function(hasBlacklistedUrls) { + if (hasBlacklistedUrls) + $('most-visited-settings').classList.add('has-blacklist'); + else + $('most-visited-settings').classList.remove('has-blacklist'); + }, + blacklist: function(el) { var self = this; var url = el.href; @@ -126,7 +133,11 @@ var MostVisited = (function() { // Send 'getMostVisitedPages' with a callback since we want to find the // new page and add that in the place of the removed page. - chromeSend('getMostVisited', [], 'mostVisitedPages', function(data) { + chromeSend('getMostVisited', [], 'mostVisitedPages', + function(data, firstRun, hasBlacklistedUrls) { + // Update settings link. + self.updateSettingsLink(hasBlacklistedUrls); + // Find new item. var newItem; for (var i = 0; i < data.length; i++) { -- cgit v1.1