Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1965913005:
ChromeOS: Implement minumal material design OOBE. (Closed)
|
Created:
4 years, 7 months ago by Alexander Alekseev Modified:
4 years, 6 months ago CC:
chromium-reviews, dzhioev+watch_chromium.org, achuith+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChromeOS: Implement minumal material design OOBE.
This CL adds Welcome screen and command-line option to turn material design
OOBE on.
BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/4c23a6b1df568c3acaa7f4753a3977de45aaa7df
Cr-Commit-Position: refs/heads/master@{#396592}
Patch Set 1 #
Total comments: 56
Patch Set 2 : Update after review. #Patch Set 3 : Inline CSS. #
Total comments: 32
Patch Set 4 : Update after review. #
Total comments: 20
Patch Set 5 : Update after review. #Patch Set 6 : Fix presubmit. #Messages
Total messages: 49 (21 generated)
Description was changed from ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none ========== to ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
alemate@chromium.org changed reviewers: + dzhioev@chromium.org
Please review.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/1
achuith@chromium.org changed reviewers: + achuith@chromium.org, jdufault@chromium.org
Jake, can you take a look?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.html:10: <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> Import in login_non_lock_shared.html? There appears to be some polymer config stuff in there already. Otherwise, what about moving this right above the custom_elements.html so the polymer stuff is together? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.html:18: <script src="chrome://resources/js/util.js"></script> Is this being used? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:72: var firstTitle; Initialize to '', and then after the for loop unconditionally return firstTitle? '' will be treated as false in if(!firstTitle) so this change shouldn't change behavior. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:310: $('oobe-poly-welcome').SetOobePolyWelcomeCurrentLanguage( What about oobe-welcome-md? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:310: $('oobe-poly-welcome').SetOobePolyWelcomeCurrentLanguage( Rename method call to SetCurrentLanguage? What about just having a property set method then, so it is $('...').currentLanguage = Oobe.getSelectedTitle(...) https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:313: $('oobe-classic-connect').hidden = true; Just oobe-connect? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.html:21: <link rel="stylesheet" href="oobe_button.css"> Linked stylesheets are deprecated. Maybe inline the css? https://www.polymer-project.org/1.0/docs/devguide/styling.html https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.html:40: <link rel="stylesheet" href="oobe_icon_button.css"> Inline the css? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_buttons.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.js:27: focus: function() { this.$.iconButton.focus(); }, Add newlines? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_card.css (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_card.css:1: /* Copyright 2016 The Chromium Authors. All rights reserved. Leave copyright the same. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_card.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_card.html:25: <link rel="stylesheet" href="oobe_card.css"> Inline CSS or pull it into a shared module. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_poly.css (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_poly.css:1: /* Copyright 2016 The Chromium Authors. All rights reserved. Leave copyright the same. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_screen_network.html:4: <link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> newline between copyright and imports https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_screen_network.html:15: <!-- -------------------------------------------------------- --> Remove this divider. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:16: <oobe-button-next id="oobeWelcomeNextButton" on-tap="onWelcomeNextButtonClicked_"></oobe-button-next> Does this ID need to be prefixed with oobeWelcome? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:18: <div class="oobe-poly-welcome-bottom-menus flex horizontal layout justified"> Rename oobe-poly-welcome-bottom-menus to bottom-menus? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:19: <div class="oobe-poly-welcome-buttonbox layout vertical"> Same, is the prefix needed? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:21: <div id="oobePolyWelcomeCurrentLanguage"></div> Remove the oobePolyWelcome prefix? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:23: <div class="oobe-poly-welcome-buttonbox layout vertical"> Remove the oobe-poly-welcome prefix? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:25: Accessibility Can this be internationalized easily? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_welcome.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:6: is: 'oobe-poly-welcome', nit: indent https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:20: * TODO(dzhioev): Remove when fixed in Polymer. Is this still broken? The bug is almost a year old and is marked fixed. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:29: focus: nit: Place function() on the same line as the property name. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:37: onAnimationFinish_: nit: as above. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:42: isRTL_: nit: as above. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:47: isWelcomeSectionActive_: nit: as above. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:59: 'slide-' + (isRTL ? 'right' : 'left') + '-animation'; Add in the trailing }s? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/ui/webui/chr... File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/ui/webui/chr... chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:495: localized_strings->SetString("mdOobeUI", UseMDOobe() ? "on" : "off"); What about using "newOobeUI" to follow "newKioskUI"?
https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.html:10: <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> On 2016/05/11 22:07:42, jdufault wrote: > Import in login_non_lock_shared.html? There appears to be some polymer config > stuff in there already. > > Otherwise, what about moving this right above the custom_elements.html so the > polymer stuff is together? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.html:18: <script src="chrome://resources/js/util.js"></script> On 2016/05/11 22:07:43, jdufault wrote: > Is this being used? Done. (This was left from a temporary code.) https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:72: var firstTitle; On 2016/05/11 22:07:43, jdufault wrote: > Initialize to '', and then after the for loop unconditionally return firstTitle? > > '' will be treated as false in if(!firstTitle) so this change shouldn't change > behavior. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:310: $('oobe-poly-welcome').SetOobePolyWelcomeCurrentLanguage( On 2016/05/11 22:07:43, jdufault wrote: > What about oobe-welcome-md? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe.js:313: $('oobe-classic-connect').hidden = true; On 2016/05/11 22:07:43, jdufault wrote: > Just oobe-connect? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.html:21: <link rel="stylesheet" href="oobe_button.css"> On 2016/05/11 22:07:43, jdufault wrote: > Linked stylesheets are deprecated. Maybe inline the css? > > https://www.polymer-project.org/1.0/docs/devguide/styling.html Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.html:40: <link rel="stylesheet" href="oobe_icon_button.css"> On 2016/05/11 22:07:43, jdufault wrote: > Inline the css? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_buttons.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.js:27: focus: function() { this.$.iconButton.focus(); }, On 2016/05/11 22:07:43, jdufault wrote: > Add newlines? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_card.css (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_card.css:1: /* Copyright 2016 The Chromium Authors. All rights reserved. On 2016/05/11 22:07:43, jdufault wrote: > Leave copyright the same. This is new file. Are you sure we need 2015 here? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_card.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_card.html:25: <link rel="stylesheet" href="oobe_card.css"> On 2016/05/11 22:07:43, jdufault wrote: > Inline CSS or pull it into a shared module. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_poly.css (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_poly.css:1: /* Copyright 2016 The Chromium Authors. All rights reserved. On 2016/05/11 22:07:43, jdufault wrote: > Leave copyright the same. THis is a new file. Re you sure about the copyright? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_screen_network.html:4: <link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> On 2016/05/11 22:07:43, jdufault wrote: > newline between copyright and imports Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_screen_network.html:15: <!-- -------------------------------------------------------- --> On 2016/05/11 22:07:43, jdufault wrote: > Remove this divider. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:16: <oobe-button-next id="oobeWelcomeNextButton" on-tap="onWelcomeNextButtonClicked_"></oobe-button-next> On 2016/05/11 22:07:43, jdufault wrote: > Does this ID need to be prefixed with oobeWelcome? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:18: <div class="oobe-poly-welcome-bottom-menus flex horizontal layout justified"> On 2016/05/11 22:07:43, jdufault wrote: > Rename oobe-poly-welcome-bottom-menus to bottom-menus? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:19: <div class="oobe-poly-welcome-buttonbox layout vertical"> On 2016/05/11 22:07:43, jdufault wrote: > Same, is the prefix needed? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:21: <div id="oobePolyWelcomeCurrentLanguage"></div> On 2016/05/11 22:07:43, jdufault wrote: > Remove the oobePolyWelcome prefix? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:23: <div class="oobe-poly-welcome-buttonbox layout vertical"> On 2016/05/11 22:07:43, jdufault wrote: > Remove the oobe-poly-welcome prefix? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.html:25: Accessibility On 2016/05/11 22:07:43, jdufault wrote: > Can this be internationalized easily? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_welcome.js (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:6: is: 'oobe-poly-welcome', On 2016/05/11 22:07:44, jdufault wrote: > nit: indent Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:20: * TODO(dzhioev): Remove when fixed in Polymer. On 2016/05/11 22:07:44, jdufault wrote: > Is this still broken? The bug is almost a year old and is marked fixed. THhs is actually copy-paste from offline gaia screen. I am not sure I can reproduce this. Do you think we should remove this? https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:29: focus: On 2016/05/11 22:07:44, jdufault wrote: > nit: Place function() on the same line as the property name. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:37: onAnimationFinish_: On 2016/05/11 22:07:44, jdufault wrote: > nit: as above. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:42: isRTL_: On 2016/05/11 22:07:44, jdufault wrote: > nit: as above. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:47: isWelcomeSectionActive_: On 2016/05/11 22:07:44, jdufault wrote: > nit: as above. Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_welcome.js:59: 'slide-' + (isRTL ? 'right' : 'left') + '-animation'; On 2016/05/11 22:07:44, jdufault wrote: > Add in the trailing }s? Done. https://codereview.chromium.org/1965913005/diff/1/chrome/browser/ui/webui/chr... File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/ui/webui/chr... chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:495: localized_strings->SetString("mdOobeUI", UseMDOobe() ? "on" : "off"); On 2016/05/11 22:07:44, jdufault wrote: > What about using "newOobeUI" to follow "newKioskUI"? Done.
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Jacob, Pavel, friendly ping.
https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:67: /** nit: newline above here https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:69: * @param {!Object} - the same as in setupSelect() above. replace "-" with "list". https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:71: getSelectedTitle: function(list) { Do the items in |list| store any interesting data besides the title? If so, maybe have this function just return the entire entry and the caller can fetch the title from the entry itself. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (left): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:24: <link rel="stylesheet" href="gaia_button.css"> Make sure to delete this file in the patch. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:48: <link rel="stylesheet" href="gaia_icon_button.css"> Make sure to delete this file in the patch. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:12: Material design buttons for OOBE screens. What about something like: Material design button that shows a forward arrow, typically used for navigating to the next page/screen. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:18: <oobe-button type="dialog"></oobe-button> Rename to oobe-next-button, remove unused type property. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:20: <dom-module id="oobe-button-next"> Rename to oobe-next-button to be consistent with oobe-icon-button. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:38: Material desing icon button with a special styling. What about: Material design button that shows an icon. nit: desing->design https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_card.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:14: <div class="header" flex vertical layout end-justified start> Is the end quote after header supposed to be after start? https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:48: .oobe-footer { Merge .oobe-footer classes https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:94: <div class="oobe-header vertical layout relative"> Can you verify that the vertical, layout, and relative classes are still defined and useful here? https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_screen_network.html:17: <oobe-welcome-md id="oobe-welcome-md" class="fit show"></oobe-welcome-md> Please verify the fit and show classes are still defined in polymer 1.x. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:34: <neon-animated-pages id="animatedPages" class="fit" attr-for-selected="id" Does the fit class still do anything? https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:41: <div class="header flex vertical layout end-justified start"> Please verify these classes are still defined and useful.
https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:67: /** On 2016/05/20 18:27:27, jdufault wrote: > nit: newline above here Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:69: * @param {!Object} - the same as in setupSelect() above. On 2016/05/20 18:27:26, jdufault wrote: > replace "-" with "list". Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe.js:71: getSelectedTitle: function(list) { On 2016/05/20 18:27:26, jdufault wrote: > Do the items in |list| store any interesting data besides the title? If so, > maybe have this function just return the entire entry and the caller can fetch > the title from the entry itself. It also contains data to setup input element, but I believe it it useless for one item. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (left): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:24: <link rel="stylesheet" href="gaia_button.css"> On 2016/05/20 18:27:27, jdufault wrote: > Make sure to delete this file in the patch. gaia_button.css is still used in gaia_buttons.html . I am not modifying offline Gaia here. I just copied it. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:48: <link rel="stylesheet" href="gaia_icon_button.css"> On 2016/05/20 18:27:27, jdufault wrote: > Make sure to delete this file in the patch. gaia_icon_button.css is still used in gaia_buttons.html . https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:12: Material design buttons for OOBE screens. On 2016/05/20 18:27:27, jdufault wrote: > What about something like: > > Material design button that shows a forward arrow, typically used for navigating > to the next page/screen. Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:18: <oobe-button type="dialog"></oobe-button> On 2016/05/20 18:27:27, jdufault wrote: > Rename to oobe-next-button, remove unused type property. Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:20: <dom-module id="oobe-button-next"> On 2016/05/20 18:27:27, jdufault wrote: > Rename to oobe-next-button to be consistent with oobe-icon-button. Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:38: Material desing icon button with a special styling. On 2016/05/20 18:27:27, jdufault wrote: > What about: > > Material design button that shows an icon. > > nit: desing->design Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_card.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:14: <div class="header" flex vertical layout end-justified start> On 2016/05/20 18:27:27, jdufault wrote: > Is the end quote after header supposed to be after start? Fixed. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:48: .oobe-footer { On 2016/05/20 18:27:27, jdufault wrote: > Merge .oobe-footer classes Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_card.html:94: <div class="oobe-header vertical layout relative"> On 2016/05/20 18:27:27, jdufault wrote: > Can you verify that the vertical, layout, and relative classes are still defined > and useful here? Yes, they are still usefull. https://elements.polymer-project.org/guides/flex-layout https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_screen_network.html:17: <oobe-welcome-md id="oobe-welcome-md" class="fit show"></oobe-welcome-md> On 2016/05/20 18:27:27, jdufault wrote: > Please verify the fit and show classes are still defined in polymer 1.x. Done. https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:34: <neon-animated-pages id="animatedPages" class="fit" attr-for-selected="id" On 2016/05/20 18:27:27, jdufault wrote: > Does the fit class still do anything? I believe so: https://elements.polymer-project.org/guides/flex-layout#general-purpose-rules https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:41: <div class="header flex vertical layout end-justified start"> On 2016/05/20 18:27:27, jdufault wrote: > Please verify these classes are still defined and useful. Done.
https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (left): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:48: <link rel="stylesheet" href="gaia_icon_button.css"> On 2016/05/24 00:38:45, Alexander Alekseev wrote: > On 2016/05/20 18:27:27, jdufault wrote: > > Make sure to delete this file in the patch. > > gaia_icon_button.css is still used in gaia_buttons.html . Is gaia_buttons.html going to go away? Otherwise the CSS should be shared by creating a style module [1]. 1: https://www.polymer-project.org/1.0/docs/devguide/styling#style-modules
https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (left): https://codereview.chromium.org/1965913005/diff/40001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:48: <link rel="stylesheet" href="gaia_icon_button.css"> On 2016/05/24 17:18:37, jdufault wrote: > On 2016/05/24 00:38:45, Alexander Alekseev wrote: > > On 2016/05/20 18:27:27, jdufault wrote: > > > Make sure to delete this file in the patch. > > > > gaia_icon_button.css is still used in gaia_buttons.html . > > Is gaia_buttons.html going to go away? Otherwise the CSS should be shared by > creating a style module [1]. > > 1: https://www.polymer-project.org/1.0/docs/devguide/styling#style-modules I believe Gaia has its own design, which doesn't depend on our OOBE design. It seems to me that we should tie only dependent projects.
dzhioev@google.com changed reviewers: + dzhioev@google.com
https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/login/oobe_buttons.html:21: <link rel="stylesheet" href="oobe_button.css"> On 2016/05/11 22:07:43, jdufault wrote: > Linked stylesheets are deprecated. Maybe inline the css? > > https://www.polymer-project.org/1.0/docs/devguide/styling.html Linked stylesheets are OK in Chrome code, because they are inlined by GRIT during compilation. I prefer to leave CSS in a separate files. Otherwise CSS style checks are not launched. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:42: <oobe-icon-button icon="close"></oobe-icon> nit: opening and closing tags are different https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.js (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.js:13: }, nit: remove ',' https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.js:31: onClick_: function(e) { You need to make the same for 'oobe-next-button' https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_screen_network.html:13: Please move these imports to the HTML file with 'oobe-welcome-md' definition. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:11: paper-dialog { not used https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:34: <neon-animated-pages id="animatedPages" class="fit" attr-for-selected="id" We don't need animations yet. Please remove. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.js (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:19: ready: function() { nit: indentation here and in the functions below. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:22: * https://github.com/PolymerElements/neon-animation/issues/32 This workaround is not needed anymore. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:64: }, nit: remove ',' https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:66: }); nit: indentation
https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.html:42: <oobe-icon-button icon="close"></oobe-icon> On 2016/05/25 01:01:32, dzhioev_at_google wrote: > nit: opening and closing tags are different Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_buttons.js (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.js:13: }, On 2016/05/25 01:01:32, dzhioev_at_google wrote: > nit: remove ',' Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_buttons.js:31: onClick_: function(e) { On 2016/05/25 01:01:32, dzhioev_at_google wrote: > You need to make the same for 'oobe-next-button' Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_screen_network.html:13: On 2016/05/25 01:01:32, dzhioev_at_google wrote: > Please move these imports to the HTML file with 'oobe-welcome-md' definition. Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:11: paper-dialog { On 2016/05/25 01:01:32, dzhioev_at_google wrote: > not used Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.html:34: <neon-animated-pages id="animatedPages" class="fit" attr-for-selected="id" On 2016/05/25 01:01:32, dzhioev_at_google wrote: > We don't need animations yet. Please remove. Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... File chrome/browser/resources/chromeos/login/oobe_welcome.js (right): https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:19: ready: function() { On 2016/05/25 01:01:32, dzhioev_at_google wrote: > nit: indentation here and in the functions below. Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:22: * https://github.com/PolymerElements/neon-animation/issues/32 On 2016/05/25 01:01:32, dzhioev_at_google wrote: > This workaround is not needed anymore. Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:64: }, On 2016/05/25 01:01:32, dzhioev_at_google wrote: > nit: remove ',' Done. https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... chrome/browser/resources/chromeos/login/oobe_welcome.js:66: }); On 2016/05/25 01:01:32, dzhioev_at_google wrote: > nit: indentation Done.
On 2016/05/25 23:47:30, Alexander Alekseev wrote: > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > File chrome/browser/resources/chromeos/login/oobe_buttons.html (right): > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_buttons.html:42: <oobe-icon-button > icon="close"></oobe-icon> > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > nit: opening and closing tags are different > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > File chrome/browser/resources/chromeos/login/oobe_buttons.js (right): > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_buttons.js:13: }, > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > nit: remove ',' > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_buttons.js:31: onClick_: > function(e) { > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > You need to make the same for 'oobe-next-button' > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > File chrome/browser/resources/chromeos/login/oobe_screen_network.html (right): > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_screen_network.html:13: > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > Please move these imports to the HTML file with 'oobe-welcome-md' definition. > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > File chrome/browser/resources/chromeos/login/oobe_welcome.html (right): > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.html:11: paper-dialog { > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > not used > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.html:34: > <neon-animated-pages id="animatedPages" class="fit" attr-for-selected="id" > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > We don't need animations yet. Please remove. > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > File chrome/browser/resources/chromeos/login/oobe_welcome.js (right): > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.js:19: ready: function() { > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > nit: indentation here and in the functions below. > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.js:22: * > https://github.com/PolymerElements/neon-animation/issues/32 > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > This workaround is not needed anymore. > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.js:64: }, > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > nit: remove ',' > > Done. > > https://codereview.chromium.org/1965913005/diff/60001/chrome/browser/resource... > chrome/browser/resources/chromeos/login/oobe_welcome.js:66: }); > On 2016/05/25 01:01:32, dzhioev_at_google wrote: > > nit: indentation > > Done. LGTM
Description was changed from ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
dzhioev@chromium.org changed reviewers: - dzhioev@google.com
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by alemate@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
Presubmit script requires update of existing code. PTAL.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/100001
On 2016/05/27 00:19:28, Alexander Alekseev wrote: > Presubmit script requires update of existing code. PTAL. LGTM
The CQ bit was unchecked by alemate@chromium.org
The CQ bit was checked by alemate@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_clang on tryserver.chromium.win (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by alemate@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1965913005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1965913005/100001
Message was sent while issue was closed.
Description was changed from ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== ChromeOS: Implement minumal material design OOBE. This CL adds Welcome screen and command-line option to turn material design OOBE on. BUG=604119 TEST=none CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/4c23a6b1df568c3acaa7f4753a3977de45aaa7df Cr-Commit-Position: refs/heads/master@{#396592} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/4c23a6b1df568c3acaa7f4753a3977de45aaa7df Cr-Commit-Position: refs/heads/master@{#396592} |
