chrome/browser/resources/ntp4/new_tab.html - Issue 9838064: Add a sign-in promo message to the Other Devices menu.

Side by Side Diff: chrome/browser/resources/ntp4/new_tab.html

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 previously-unaddressed nits. 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 unified diff | Download patch | Annotate | Revision Log

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <html i18n-values="	2 <html i18n-values="

3 dir:textdirection;	3 dir:textdirection;

4 hasattribution:hasattribution;	4 hasattribution:hasattribution;

5 customlogo:customlogo;	5 customlogo:customlogo;

6 themegravity:themegravity;	6 themegravity:themegravity;

7 bookmarkbarattached:bookmarkbarattached;"	7 bookmarkbarattached:bookmarkbarattached;"

8 class="starting-up">	8 class="starting-up">

9 <head>	9 <head>

10 <meta charset="utf-8">	10 <meta charset="utf-8">

(...skipping 92 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
103 <div id="footer-content">	103 <div id="footer-content">

104 <img id="logo-img">	104 <img id="logo-img">

105	105

106 <ul id="dot-list">	106 <ul id="dot-list">

107 </ul>	107 </ul>

108	108

109 <div class="menu-container">	109 <div class="menu-container">

110 <button id="other-sessions-menu-button"	110 <button id="other-sessions-menu-button"

111 class="footer-menu-button custom-appearance invisible">	111 class="footer-menu-button custom-appearance invisible">

112 <span i18n-content="otherSessions"></span>	112 <span i18n-content="otherSessions"></span>

113 <div class="disclosure-triangle"></div>	113 <div class="disclosure-triangle"></div>
	Dan Beam 2012/03/27 01:21:06 might be better for promo message to live inside b might be better for promo message to live inside but [hidden], see below Patrick Dubroy 2012/03/27 13:44:21 I'm not sure. The thing is, the promo message even Show quoted text On 2012/03/27 01:21:06, Dan Beam wrote: > might be better for promo message to live inside but [hidden], see below I'm not sure. The thing is, the promo message eventually lives inside the <menu> element, which is created dynamically. If I put it here, it brings the two pieces of related code close together, but I don't think it really makes sense for the promo message to be a child of the button. I'd just end up having to reparent it to make it a child of the menu. Meta-review: when you use words like "might" or "maybe" in your comments, it's hard for me to figure out whether you'd like me to change it, or whether you are leaving it up to me to decide. Dan Beam 2012/03/27 18:09:21 Cloning a node from the template section like we t Show quoted text On 2012/03/27 13:44:21, dubroy wrote: > On 2012/03/27 01:21:06, Dan Beam wrote: > > might be better for promo message to live inside but [hidden], see below > > I'm not sure. The thing is, the promo message eventually lives inside the <menu> > element, which is created dynamically. If I put it here, it brings the two > pieces of related code close together, but I don't think it really makes sense > for the promo message to be a child of the button. I'd just end up having to > reparent it to make it a child of the menu. Cloning a node from the template section like we talked about seems fine as long as the menu owns that node. Show quoted text > Meta-review: when you use words like "might" or "maybe" in your comments, it's > hard for me to figure out whether you'd like me to change it, or whether you are > leaving it up to me to decide. We talked about this and maybe/might means I'm asking for your input as well.
114 </button>	114 </button>

115	115

116 <button id="recently-closed-menu-button"	116 <button id="recently-closed-menu-button"

117 class="footer-menu-button custom-appearance">	117 class="footer-menu-button custom-appearance">

118 <span i18n-content="recentlyclosed"></span>	118 <span i18n-content="recentlyclosed"></span>

119 <div class="disclosure-triangle"></div>	119 <div class="disclosure-triangle"></div>

120 </button>	120 </button>

121	121

122 <div id="vertical-separator"></div>	122 <div id="vertical-separator"></div>

123 </div>	123 </div>

(...skipping 66 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
190 <!-- App install hint -->	190 <!-- App install hint -->

191 <a id="app-install-hint-template" class="app-install-hint">	191 <a id="app-install-hint-template" class="app-install-hint">

192 <div class="app-install-hint-interior">	192 <div class="app-install-hint-interior">

193 <div class="interior-background"></div>	193 <div class="interior-background"></div>

194 <div class="vertical-bar"></div>	194 <div class="vertical-bar"></div>

195 <div class="horizontal-bar"></div>	195 <div class="horizontal-bar"></div>

196 </div>	196 </div>

197 <span i18n-content="appInstallHintText"></span>	197 <span i18n-content="appInstallHintText"></span>

198 </a>	198 </a>

199	199

	200 <!-- Message shown in the other sessions menu when the user is signed in but

	201 there is no session data (e.g. they have tab sync turned off). -->
	Dan Beam 2012/03/27 01:21:06 btw, estade brought up a good point that if you're btw, estade brought up a good point that if you're not copying this multiple times maybe it should live above ^ Patrick Dubroy 2012/03/27 13:44:21 See my comment above. I don't think it makes any m Show quoted text On 2012/03/27 01:21:06, Dan Beam wrote: > btw, estade brought up a good point that if you're not copying this multiple > times maybe it should live above ^ See my comment above. I don't think it makes any more sense to put it there.
	202 <div id="other-sessions-promo-message" class="promo-message">

	203 <span i18n-content="otherSessionsEmpty"></span>

	204 <p>

	205 <a i18n-values="href:otherSessionsLearnMoreUrl" i18n-content="learnMore">

	206 </a>

	207 </p>

	208 </div>

	209

200 </div>	210 </div>

201	211

202 <!-- This is used to measure text in the current locale. It is not visible. -->	212 <!-- This is used to measure text in the current locale. It is not visible. -->

203 <div id="fontMeasuringDiv"></div>	213 <div id="fontMeasuringDiv"></div>

204	214

205 </html>	215 </html>

OLD	NEW

« no previous file with comments | « chrome/browser/resources/ntp4/footer_menu.css ('k') | chrome/browser/resources/ntp4/new_tab.js » ('j') | chrome/browser/resources/ntp4/new_tab.js » ('J')