chrome/browser/resources/settings/date_time_page/date_time_page.html - Issue 990473002: Add initial implementation of date-time-settings MD settings page.

Side by Side Diff: chrome/browser/resources/settings/date_time_page/date_time_page.html

Issue 990473002: Add initial implementation of date-time-settings MD settings page. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 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

« no previous file with comments | « chrome/browser/resources/settings/date_time_page/date_time_page.css ('k') | chrome/browser/resources/settings/date_time_page/date_time_page.js » ('j') | chrome/browser/resources/settings/prefs/prefs_types.js » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <link rel="import" href="chrome://md-settings/checkbox/checkbox.html">

	2 <link rel="import" href="chrome://resources/polymer/polymer/polymer.html">

	3 <link rel="import" href="chrome://resources/polymer/core-label/core-label.html">

	4 <link rel="import" href="chrome://resources/polymer/core-menu/core-menu.html">

	5 <link rel="import" href="chrome://resources/polymer/paper-item/paper-item.html">

	6 <link rel="import" href="chrome://resources/polymer/paper-shadow/paper-shadow.ht ml">

	7 <link rel="import" href="chrome://resources/cr_elements/cr_dropdown_menu/

	8 cr_dropdown_menu.html">

	9

	10 <polymer-element name="cr-settings-date-time-page">

	11 <template>

	12 <link rel="stylesheet" href="chrome://md-settings/settings_page/settings_pag e.css">

	13 <link rel="stylesheet" href="date_time_page.css">

	14 <paper-shadow layout vertical cross-fade>

	15 <cr-settings-page-header page="{{}}"></cr-settings-page-header>

	16 <div horizontal layout center>

	17 <core-label horizontal layout center>

	18 <span class="time-zone-label">Time zone:</span>

	19 <cr-dropdown-menu id="timezoneMenu">

	20 <core-menu class="menu" valueAttr="value" selectedAttribute=""

	21 selected="{{prefs.settings.cros.system.timezone}}">

	22 <paper-item value="America/Los_Angeles">

	23 Pacific Standard Time

	24 </paper-item>

	25 <paper-item value="America/New_York">

	26 Eastern Standard Time

	27 </paper-item>

	28 </core-menu>
	stevenjb 2015/03/25 21:15:30 The above seems pretty heavyweight for a dropdown The above seems pretty heavyweight for a dropdown and a label. Presumably in this case we will eventually want the contents (i.e. list of time zones) to be data driven. I expect that will be generally true for other dropdowns. I'd kind of prefer we implement this without the core-menu in the HTML and add the test entries in the JS, even in this initial implementation. Jeremy Klein 2015/03/25 21:24:26 This will eventually be done in a cr-settings-sele Show quoted text On 2015/03/25 21:15:30, stevenjb wrote: > The above seems pretty heavyweight for a dropdown and a label. Presumably in > this case we will eventually want the contents (i.e. list of time zones) to be > data driven. I expect that will be generally true for other dropdowns. > > I'd kind of prefer we implement this without the core-menu in the HTML and add > the test entries in the JS, even in this initial implementation. This will eventually be done in a cr-settings-select or something, which will have some attribute which is a list of options (or map from text to value). You're right that this will be data-driven via a <template repeat="blah">, but I'm not sure I understand your request to implement this without a core-menu. I'm pretty sure we'll need a core-menu regardless of whether it's here or inside the cr-settings-select. I also think it's fine to land this as-is until we have the cr-settings-select and some idea of the structure of the timezone objects and the api we get those from. It would be fine to do that in a followup to the basic UI IMO. stevenjb 2015/03/25 21:55:36 I was suggesting that it should be inside cr-setti Show quoted text On 2015/03/25 21:24:26, Jeremy Klein wrote: > On 2015/03/25 21:15:30, stevenjb wrote: > > The above seems pretty heavyweight for a dropdown and a label. Presumably in > > this case we will eventually want the contents (i.e. list of time zones) to be > > data driven. I expect that will be generally true for other dropdowns. > > > > I'd kind of prefer we implement this without the core-menu in the HTML and add > > the test entries in the JS, even in this initial implementation. > > This will eventually be done in a cr-settings-select or something, which will > have some attribute which is a list of options (or map from text to value). > You're right that this will be data-driven via a <template repeat="blah">, but > I'm not sure I understand your request to implement this without a core-menu. > I'm pretty sure we'll need a core-menu regardless of whether it's here or inside > the cr-settings-select. I also think it's fine to land this as-is until we have > the cr-settings-select and some idea of the structure of the timezone objects > and the api we get those from. It would be fine to do that in a followup to the > basic UI IMO. I was suggesting that it should be inside cr-settings-select. At the page level a dropdown should (ideally, imho) appear as a single element with just the id, pref, and label attributes. I don't have a strong objection to landing this as-is, but personally I would prefer we omit this for now so that somebody doesn't unknowingly copy this pattern and use it elsewhere. Unlikely, granted, but requiring a dozen or so lines of HTML for what should be a single element is just the sort of pattern I am hoping we can avoid.
	29 </cr-dropdown-menu>

	30 </core-label>

	31 </div>

	32 <cr-settings-checkbox

	33 pref="{{prefs.settings.settings.clock.use_24Hour_clock}}"

	34 label="Use 24-hour clock">

	35 </cr-settings-checkbox>

	36 <span id="setAutomatically">Date and time are set automatically.</span>

	37 </paper-shadow>

	38 </template>

	39 <script src="date_time_page.js"></script>

	40 </polymer-element>

OLD	NEW