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

Unified Diff: chrome/browser/resources/ntp4/other_sessions.js

Issue 9838064: Add a sign-in promo message to the Other Devices menu. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments, and only enable menu when tab sync is enabled. Created 8 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/resources/ntp4/other_sessions.js
diff --git a/chrome/browser/resources/ntp4/other_sessions.js b/chrome/browser/resources/ntp4/other_sessions.js
index ccd693d67b8f1385a813ace1e3733ac51e0199bf..64aa0cc0916f16c2eca3afff54b47ae83fd3ecf6 100644
--- a/chrome/browser/resources/ntp4/other_sessions.js
+++ b/chrome/browser/resources/ntp4/other_sessions.js
@@ -24,6 +24,11 @@ cr.define('ntp', function() {
};
var HISTOGRAM_EVENT_LIMIT = HISTOGRAM_EVENT.LINK_RIGHT_CLICKED + 1;
+ // Indicates whether or not the current user is signed in. This is
+ // initially set in initialize(), and is updated when the user's login state
+ // changes.
+ var isUserSignedIn = false;
Dan Beam 2012/03/26 23:18:54 nit: this is probably meant to go on the class pro
Patrick Dubroy 2012/03/27 00:36:51 Actually, it's not even used anymore.
+
OtherSessionsMenuButton.prototype = {
__proto__: MenuButton.prototype,
@@ -36,15 +41,24 @@ cr.define('ntp', function() {
this.onContextMenu_.bind(this), true);
document.body.appendChild(this.menu);
+ this.resetMenu_();
+
this.sessions_ = [];
this.anchorType = cr.ui.AnchorType.ABOVE;
this.invertLeftRight = true;
- chrome.send('getForeignSessions');
this.recordUmaEvent_(HISTOGRAM_EVENT.INITIALIZED);
},
/**
+ * Initialize this element.
+ * @param {boolean} signedIn Is the current user signed in?
+ */
+ initialize: function(signedIn) {
+ this.updateSignInState(signedIn);
+ },
+
+ /**
* Record an event in the UMA histogram.
* @param {Number} eventId The id of the event to be recorded.
*/
@@ -76,6 +90,17 @@ cr.define('ntp', function() {
},
/**
+ * Reset the menu to the default state.
+ * @private
+ */
+ resetMenu_: function() {
+ var promoContent = $('other-sessions-promo-message');
Dan Beam 2012/03/26 23:18:54 why do you need to do var promoContent = $('oth
Patrick Dubroy 2012/03/26 23:46:07 No, 'this' is the button that you click to show th
Dan Beam 2012/03/27 00:03:42 Then it seems like you should be assigning this to
Patrick Dubroy 2012/03/27 00:36:51 Done.
+ this.menu.innerHTML = '';
+ this.menu.appendChild(promoContent);
+ promoContent.hidden = true;
+ },
+
+ /**
* Create a custom click handler for a link, so that clicking on a link
* restores the session (including back stack) rather than just opening
* the URL.
@@ -122,35 +147,53 @@ cr.define('ntp', function() {
},
/**
- * Create the UI for the promo and place it inside the menu.
- * The promo is shown instead of foreign session data when tab sync is
- * not enabled for a profile.
+ * Show or hide the promo message shown in place of the session list.
+ * The promo is shown when the user is signed in, but there is no foreign
+ * session data available.
+ * @param {boolean} visible Whether the message should be shown.
+ * @private
*/
- showPromo_: function() {
- var message = localStrings.getString('otherSessionsEmpty');
- this.menu.appendChild(this.ownerDocument.createTextNode(message));
+ setPromoVisible_: function(visible) {
+ this.resetMenu_();
+ $('other-sessions-promo-message').hidden = !visible;
},
/**
- * Sets the menu model data.
+ * Sets the menu model data. An empty list means that either there are no
+ * foreign sessions, or tab sync is disabled for this profile.
+ * |isTabSyncEnabled| makes it possible to distinguish between the cases.
+ *
* @param {Array} sessionList Array of objects describing the sessions
- * from other devices.
+ * from other devices.
+ * @param {boolean} isTabSyncEnabled Is tab sync enabled for this profile?
*/
- set sessions(sessionList) {
- // Clear the current contents of the menu.
- this.menu.innerHTML = '';
-
- // Rebuild the menu with the new data.
- for (var i = 0; i < sessionList.length; i++) {
- this.addSession_(sessionList[i]);
+ setForeignSessions: function(sessionList, isTabSyncEnabled) {
+ this.sessions_ = sessionList;
+ this.setPromoVisible_(sessionList.length == 0);
+ if (sessionList.length > 0) {
+ // Rebuild the menu with the new data.
+ for (var i = 0; i < sessionList.length; i++) {
+ this.addSession_(sessionList[i]);
+ }
}
-
- if (sessionList.length == 0)
- this.classList.add('invisible');
- else
+ // The menu button is shown iff tab sync is enabled.
+ if (isTabSyncEnabled)
this.classList.remove('invisible');
+ else
+ this.classList.add('invisible');
+ },
- this.sessions_ = sessionList;
+ /**
+ * Called from the new tab page when the user's signed in state changes.
+ * @param {boolean} signedIn Is the user currently signed in?
+ */
+ updateSignInState: function(signedIn) {
+ isUserSignedIn = signedIn;
+
+ if (signedIn)
+ chrome.send('getForeignSessions');
+ else
+ this.classList.add('invisible');
},
};

Powered by Google App Engine
This is Rietveld 408576698