Commit fc94bd1e authored by Hector Carmona's avatar Hector Carmona Committed by Commit Bot

Navi: Revert bookmark bar state back to normal if users don't want navi.

Both the email and google app modules will show the bookmark bar. This
change makes it so that the state of the bookmark bar is returned to the
way it was before it was shown if no bookmarks are added.

R=scottchen@chromium.org

Bug: 881937
Change-Id: I64d0a03f6747589d594720dad406e14ee4483ca8
Reviewed-on: https://chromium-review.googlesource.com/c/1302316
Commit-Queue: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: default avatarScott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603761}
parent 0146a2a3
...@@ -13,6 +13,7 @@ js_type_check("closure_compile") { ...@@ -13,6 +13,7 @@ js_type_check("closure_compile") {
js_library("nux_email") { js_library("nux_email") {
deps = [ deps = [
"../:navigation_behavior",
"../shared:nux_types", "../shared:nux_types",
] ]
} }
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
<link rel="import" href="../navigation_behavior.html"> <link rel="import" href="../navigation_behavior.html">
<link rel="import" href="../shared/bookmark_proxy.html"> <link rel="import" href="../shared/bookmark_proxy.html">
<link rel="import" href="../shared/chooser_shared_css.html"> <link rel="import" href="../shared/chooser_shared_css.html">
<link rel="import" href="../shared/i18n_setup.html">
<link rel="import" href="../shared/step_indicator.html"> <link rel="import" href="../shared/step_indicator.html">
<link rel="import" href="nux_email_proxy.html"> <link rel="import" href="nux_email_proxy.html">
......
...@@ -27,11 +27,7 @@ Polymer({ ...@@ -27,11 +27,7 @@ Polymer({
*/ */
emailList_: Array, emailList_: Array,
/** @private */ bookmarkBarWasShown: Boolean,
bookmarkBarWasShown_: {
type: Boolean,
value: loadTimeData.getBoolean('bookmark_bar_shown'),
},
/** @private */ /** @private */
finalized_: Boolean, finalized_: Boolean,
...@@ -165,7 +161,7 @@ Polymer({ ...@@ -165,7 +161,7 @@ Polymer({
this.selectedEmailProvider_.bookmarkId = results.id; this.selectedEmailProvider_.bookmarkId = results.id;
}); });
} else { } else {
this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown_); this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
} }
// Announcements are mutually exclusive, so keeping separate. // Announcements are mutually exclusive, so keeping separate.
...@@ -182,7 +178,7 @@ Polymer({ ...@@ -182,7 +178,7 @@ Polymer({
onNoThanksClicked_: function() { onNoThanksClicked_: function() {
this.finalized_ = true; this.finalized_ = true;
this.revertBookmark_(); this.revertBookmark_();
this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown_); this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
this.emailProxy_.recordNoThanks(); this.emailProxy_.recordNoThanks();
welcome.navigateToNextStep(); welcome.navigateToNextStep();
}, },
......
<link rel="import" href="chrome://resources/html/polymer.html"> <link rel="import" href="chrome://resources/html/polymer.html">
<link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html"> <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
<link rel="import" href="../navigation_behavior.html">
<link rel="import" href="../shared/bookmark_proxy.html">
<link rel="import" href="email_chooser.html"> <link rel="import" href="email_chooser.html">
<dom-module id="nux-email"> <dom-module id="nux-email">
...@@ -40,4 +42,4 @@ ...@@ -40,4 +42,4 @@
</div> </div>
</template> </template>
<script src="nux_email.js"></script> <script src="nux_email.js"></script>
</dom-module> </dom-module>
\ No newline at end of file
...@@ -5,8 +5,25 @@ ...@@ -5,8 +5,25 @@
Polymer({ Polymer({
is: 'nux-email', is: 'nux-email',
behaviors: [welcome.NavigationBehavior],
properties: { properties: {
/** @type {nux.stepIndicatorModel} */ /** @type {nux.stepIndicatorModel} */
indicatorModel: Object, indicatorModel: Object,
}, },
/**
* Elements can override onRouteChange to handle route changes.
* Overrides function in behavior.
* @param {!welcome.Routes} route
* @param {number} step
*/
onRouteChange: function(route, step) {
if (`step-${step}` == this.id) {
nux.BookmarkProxyImpl.getInstance().isBookmarkBarShown().then(
bookmarkBarShown => {
this.$.emailChooser.bookmarkBarWasShown = bookmarkBarShown;
});
}
},
}); });
...@@ -133,10 +133,9 @@ cr.define('nux', function() { ...@@ -133,10 +133,9 @@ cr.define('nux', function() {
/** @override */ /** @override */
recordProviderSelected(providerId, length) { recordProviderSelected(providerId, length) {
this.savedProvider_ = providerId; this.savedProvider_ = providerId;
// TODO(hcarmona): get enum's max from loadTimeData instead, since length
// might not be accurate once we start localizing.
chrome.metricsPrivate.recordEnumerationValue( chrome.metricsPrivate.recordEnumerationValue(
SELECTION_METRIC_NAME, providerId, length); SELECTION_METRIC_NAME, providerId,
loadTimeData.getInteger('email_providers_count'));
} }
/** @override */ /** @override */
......
...@@ -34,6 +34,8 @@ Polymer({ ...@@ -34,6 +34,8 @@ Polymer({
*/ */
appList_: Array, appList_: Array,
bookmarkBarWasShown: Boolean,
hasAppsSelected: { hasAppsSelected: {
type: Boolean, type: Boolean,
notify: true, notify: true,
...@@ -62,22 +64,20 @@ Polymer({ ...@@ -62,22 +64,20 @@ Polymer({
// Default select all items. // Default select all items.
app.selected = true; app.selected = true;
this.updateBookmark(app); this.updateBookmark(app);
// Icons only need to be added to the cache once.
this.appsProxy_.cacheBookmarkIcon(app.id);
}); });
}); });
} }
}, },
/** /** Called when bookmarks should be removed for all selected apps. */
* Returns an array of booleans for each selected app. removeAllBookmarks() {
* @return {!Array<boolean>} this.appList_.forEach(app => {
*/ if (app.selected) {
getSelectedAppList() { app.selected = false;
if (this.appList_) this.updateBookmark(app);
return this.appList_.map(a => a.selected); }
else });
return []; this.updateHasAppsSelected();
}, },
/** /**
...@@ -96,6 +96,8 @@ Polymer({ ...@@ -96,6 +96,8 @@ Polymer({
result => { result => {
item.bookmarkId = result.id; item.bookmarkId = result.id;
}); });
// Cache bookmark icon.
this.appsProxy_.cacheBookmarkIcon(item.id);
} else if (!item.selected && item.bookmarkId) { } else if (!item.selected && item.bookmarkId) {
this.bookmarkProxy_.removeBookmark(item.bookmarkId); this.bookmarkProxy_.removeBookmark(item.bookmarkId);
item.bookmarkId = null; item.bookmarkId = null;
...@@ -111,7 +113,7 @@ Polymer({ ...@@ -111,7 +113,7 @@ Polymer({
let item = e.model.item; let item = e.model.item;
e.model.set('item.selected', !item.selected); e.model.set('item.selected', !item.selected);
this.updateBookmark(item); this.updateBookmark(item);
this.hasAppsSelected = this.computeHasAppsSelected_(); this.updateHasAppsSelected();
}, },
/** /**
...@@ -131,10 +133,12 @@ Polymer({ ...@@ -131,10 +133,12 @@ Polymer({
}, },
/** /**
* @return {boolean} * Updates the value of hasAppsSelected.
* @private * @private
*/ */
computeHasAppsSelected_: function() { updateHasAppsSelected: function() {
return this.appList_ && this.appList_.some(a => a.selected); this.hasAppsSelected = this.appList_ && this.appList_.some(a => a.selected);
if (!this.hasAppsSelected)
this.bookmarkProxy_.toggleBookmarkBar(this.bookmarkBarWasShown);
}, },
}); });
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
<link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html"> <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html">
<link rel="import" href="../navigation_behavior.html"> <link rel="import" href="../navigation_behavior.html">
<link rel="import" href="../shared/bookmark_proxy.html">
<link rel="import" href="../shared/step_indicator.html"> <link rel="import" href="../shared/step_indicator.html">
<link rel="import" href="apps_chooser.html"> <link rel="import" href="apps_chooser.html">
<link rel="import" href="nux_google_apps_proxy.html"> <link rel="import" href="nux_google_apps_proxy.html">
......
...@@ -22,13 +22,19 @@ Polymer({ ...@@ -22,13 +22,19 @@ Polymer({
* @param {number} step * @param {number} step
*/ */
onRouteChange: function(route, step) { onRouteChange: function(route, step) {
if (`step-${step}` == this.id) if (`step-${step}` == this.id) {
nux.BookmarkProxyImpl.getInstance().isBookmarkBarShown().then(
bookmarkBarShown => {
this.$.appChooser.bookmarkBarWasShown = bookmarkBarShown;
});
this.$.appChooser.populateAllBookmarks(); this.$.appChooser.populateAllBookmarks();
}
}, },
/** @private */ /** @private */
onNoThanksClicked_: function() { onNoThanksClicked_: function() {
// TODO(hcarmona): Add metrics. // TODO(hcarmona): Add metrics.
this.$.appChooser.removeAllBookmarks();
welcome.navigateToNextStep(); welcome.navigateToNextStep();
}, },
......
...@@ -25,6 +25,9 @@ cr.define('nux', function() { ...@@ -25,6 +25,9 @@ cr.define('nux', function() {
/** @param {boolean} show */ /** @param {boolean} show */
toggleBookmarkBar(show) {} toggleBookmarkBar(show) {}
/** @return {!Promise<boolean>} */
isBookmarkBarShown() {}
} }
/** @implements {nux.BookmarkProxy} */ /** @implements {nux.BookmarkProxy} */
...@@ -43,6 +46,11 @@ cr.define('nux', function() { ...@@ -43,6 +46,11 @@ cr.define('nux', function() {
toggleBookmarkBar(show) { toggleBookmarkBar(show) {
chrome.send('toggleBookmarkBar', [show]); chrome.send('toggleBookmarkBar', [show]);
} }
/** @override */
isBookmarkBarShown() {
return cr.sendWithPromise('isBookmarkBarShown');
}
} }
cr.addSingletonGetter(BookmarkProxyImpl); cr.addSingletonGetter(BookmarkProxyImpl);
......
...@@ -21,6 +21,11 @@ void BookmarkHandler::RegisterMessages() { ...@@ -21,6 +21,11 @@ void BookmarkHandler::RegisterMessages() {
"toggleBookmarkBar", "toggleBookmarkBar",
base::BindRepeating(&BookmarkHandler::HandleToggleBookmarkBar, base::BindRepeating(&BookmarkHandler::HandleToggleBookmarkBar,
base::Unretained(this))); base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"isBookmarkBarShown",
base::BindRepeating(&BookmarkHandler::HandleIsBookmarkBarShown,
base::Unretained(this)));
} }
void BookmarkHandler::HandleToggleBookmarkBar(const base::ListValue* args) { void BookmarkHandler::HandleToggleBookmarkBar(const base::ListValue* args) {
...@@ -29,13 +34,16 @@ void BookmarkHandler::HandleToggleBookmarkBar(const base::ListValue* args) { ...@@ -29,13 +34,16 @@ void BookmarkHandler::HandleToggleBookmarkBar(const base::ListValue* args) {
prefs_->SetBoolean(bookmarks::prefs::kShowBookmarkBar, show); prefs_->SetBoolean(bookmarks::prefs::kShowBookmarkBar, show);
} }
void BookmarkHandler::AddSources(content::WebUIDataSource* html_source, void BookmarkHandler::HandleIsBookmarkBarShown(const base::ListValue* args) {
PrefService* prefs) { AllowJavascript();
// Add constants to loadtime data
html_source->AddBoolean( CHECK_EQ(1U, args->GetSize());
"bookmark_bar_shown", const base::Value* callback_id;
prefs->GetBoolean(bookmarks::prefs::kShowBookmarkBar)); CHECK(args->Get(0, &callback_id));
html_source->SetJsonPath("strings.js");
ResolveJavascriptCallback(
*callback_id,
base::Value(prefs_->GetBoolean(bookmarks::prefs::kShowBookmarkBar)));
} }
} // namespace nux } // namespace nux
...@@ -11,10 +11,6 @@ ...@@ -11,10 +11,6 @@
class PrefService; class PrefService;
namespace content {
class WebUIDataSource;
} // namespace content
namespace nux { namespace nux {
class BookmarkHandler : public content::WebUIMessageHandler { class BookmarkHandler : public content::WebUIMessageHandler {
...@@ -27,10 +23,7 @@ class BookmarkHandler : public content::WebUIMessageHandler { ...@@ -27,10 +23,7 @@ class BookmarkHandler : public content::WebUIMessageHandler {
// Callbacks for JS APIs. // Callbacks for JS APIs.
void HandleToggleBookmarkBar(const base::ListValue* args); void HandleToggleBookmarkBar(const base::ListValue* args);
void HandleIsBookmarkBarShown(const base::ListValue* args);
// Adds webui sources.
static void AddSources(content::WebUIDataSource* html_source,
PrefService* prefs);
private: private:
// Weak reference. // Weak reference.
......
...@@ -119,6 +119,11 @@ void EmailHandler::AddSources(content::WebUIDataSource* html_source) { ...@@ -119,6 +119,11 @@ void EmailHandler::AddSources(content::WebUIDataSource* html_source) {
IDR_NUX_EMAIL_OUTLOOK_2X); IDR_NUX_EMAIL_OUTLOOK_2X);
html_source->AddResourcePath("email/yahoo_1x.png", IDR_NUX_EMAIL_YAHOO_1X); html_source->AddResourcePath("email/yahoo_1x.png", IDR_NUX_EMAIL_YAHOO_1X);
html_source->AddResourcePath("email/yahoo_2x.png", IDR_NUX_EMAIL_YAHOO_2X); html_source->AddResourcePath("email/yahoo_2x.png", IDR_NUX_EMAIL_YAHOO_2X);
// Add constants to loadtime data
html_source->AddInteger("email_providers_count",
static_cast<int>(EmailProviders::kCount));
html_source->SetJsonPath("strings.js");
} }
} // namespace nux } // namespace nux
...@@ -185,7 +185,6 @@ WelcomeUI::WelcomeUI(content::WebUI* web_ui, const GURL& url) ...@@ -185,7 +185,6 @@ WelcomeUI::WelcomeUI(content::WebUI* web_ui, const GURL& url)
// Add the shared bookmark handler for onboarding modules. // Add the shared bookmark handler for onboarding modules.
web_ui->AddMessageHandler( web_ui->AddMessageHandler(
std::make_unique<nux::BookmarkHandler>(profile->GetPrefs())); std::make_unique<nux::BookmarkHandler>(profile->GetPrefs()));
nux::BookmarkHandler::AddSources(html_source, profile->GetPrefs());
// Add email provider bookmarking onboarding module. // Add email provider bookmarking onboarding module.
web_ui->AddMessageHandler(std::make_unique<nux::EmailHandler>( web_ui->AddMessageHandler(std::make_unique<nux::EmailHandler>(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment