chrome/browser/resources/chromeos/login/oobe_reset.html - Issue 2756863002: ChromeOS: Powerwash UI update

Side by Side Diff: chrome/browser/resources/chromeos/login/oobe_reset.html

Issue 2756863002: ChromeOS: Powerwash UI update (Closed)

Patch Set: Use String to denote UI state Created 3 years, 8 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:

« chrome/browser/resources/chromeos/login/oobe_reset.css ('K') | « chrome/browser/resources/chromeos/login/oobe_reset.css ('k') | chrome/browser/resources/chromeos/login/oobe_reset.js » ('j') | chrome/browser/resources/chromeos/login/oobe_reset.js » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!-- Copyright 2017 The Chromium Authors. All rights reserved.

	2 Use of this source code is governed by a BSD-style license that can be

	3 found in the LICENSE file. -->

	4

	5 <dom-module id="oobe-reset-md">

	6 <template>

	7 <link rel="stylesheet" href="oobe_reset.css">

	8 <link rel="stylesheet" href="oobe_dialog_parameters.css">

	9 <link rel="stylesheet" href="chrome://resources/css/throbber.css">

	10 <oobe-dialog id="resetDialog" role="dialog"
	Alexander Alekseev 2017/04/08 10:36:18 The dialog size should be 768x640 px. This one is The dialog size should be 768x640 px. This one is so large that it does not fit the screen. Wenzhao (Colin) Zang 2017/04/10 20:37:35 It's changed to 768x640 px now. Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > The dialog size should be 768x640 px. > This one is so large that it does not fit the screen. It's changed to 768x640 px now.
	11 i18n-values="aria-label:resetScreenAccessibleTitle" has-buttons>

	12 <div class="header">

	13 <img src="chrome://theme/IDR_RESET_ALERT"
	Alexander Alekseev 2017/04/08 10:36:18 If you place the icon here, the header will occupy If you place the icon here, the header will occupy too much space. The icon should be passed to oobe-dialog element as oobe-icon. Alexander Alekseev 2017/04/08 10:36:18 OOBE/signin code is traditionally using embedded i OOBE/signin code is traditionally using embedded images. I don't think we really need embedded images for this screen (probably your approach is better because this screen is rarely used), but just FYI. Alexander Alekseev 2017/04/08 10:36:18 Please replace this icon with polymer icon icons:i Please replace this icon with polymer icon icons:info . Wenzhao (Colin) Zang 2017/04/10 20:37:35 Done. The info icon is black so it's different fro Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > Please replace this icon with polymer icon icons:info . Done. The info icon is black so it's different from the mock. Is it okay? If so I'll delete the image in the assets directory. Wenzhao (Colin) Zang 2017/04/10 20:37:35 Done. Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > OOBE/signin code is traditionally using embedded images. > I don't think we really need embedded images for this screen (probably your > approach is better because this screen is rarely used), but just FYI. Done. Wenzhao (Colin) Zang 2017/04/10 20:37:35 Originally I used iron-icon, but by using <iron-ic Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > If you place the icon here, the header will occupy too much space. > The icon should be passed to oobe-dialog element as oobe-icon. Originally I used iron-icon, but by using <iron-icon src="..."> I couldn't make it automatically show two resolutions, like <img src="..."> does. Wenzhao (Colin) Zang 2017/04/11 20:42:53 Now the icon is blue. And the illustration is embe Show quoted text On 2017/04/10 20:37:35, Wenzhao (Colin) Zang wrote: > On 2017/04/08 10:36:18, Alexander Alekseev wrote: > > Please replace this icon with polymer icon icons:info . > > Done. The info icon is black so it's different from the mock. Is it okay? If so > I'll delete the image in the assets directory. Now the icon is blue. And the illustration is embedded image now.
	14 i18n-values="alt:resetScreenIconTitle">

	15 <h1 class="title" i18n-content="resetWarningTitle"></h1>
	Alexander Alekseev 2017/04/08 10:36:18 We do not have precise specks for reset screen, bu We do not have precise specks for reset screen, but we usually use 13px Roboto (see specks in go/cros-oobe). Wenzhao (Colin) Zang 2017/04/10 20:37:35 Done. Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > We do not have precise specks for reset screen, but we usually use 13px Roboto > (see specks in go/cros-oobe). Done.
	16 </div>

	17 <div class="footer">

	18 <div hidden="[[isPowerwashHidden_(uiState_)]]"
	Alexander Alekseev 2017/04/11 04:19:26 We can (probably) make this more readable if we em We can (probably) make this more readable if we embed states to HTML. You can actually put constants to polymer expressions: hidden="[[!isState(uiState_,'powerwash-proposal-view')]]" Then, you can define isState() as: isState: function(actualState, state) { return state == actualState; } It looks like the only specific method left here would be isHelpLinkHidden_(). Wenzhao (Colin) Zang 2017/04/11 20:42:54 Done. Show quoted text On 2017/04/11 04:19:26, Alexander Alekseev wrote: > We can (probably) make this more readable if we embed states to HTML. > You can actually put constants to polymer expressions: > > hidden="[[!isState(uiState_,'powerwash-proposal-view')]]" > > Then, you can define isState() as: > > isState: function(actualState, state) { > return state == actualState; > } > > It looks like the only specific method left here would be isHelpLinkHidden_(). Done.
	19 i18n-content="resetPowerwashWarningDetails"></div>

	20 <div hidden="[[isRollbackHidden_(uiState_)]]"

	21 i18n-content="resetPowerwashRollbackWarningDetails"></div>

	22 <div hidden="[[isDataWarningHidden_(uiState_)]]"

	23 i18n-content="resetWarningDataDetails"></div>

	24 <div hidden="[[isRestartHidden_(uiState_)]]"

	25 i18n-content="resetRestartMessage"></div>

	26 <a id="powerwash-help-link-md" href="#"
	Alexander Alekseev 2017/04/08 10:36:18 Are you sure that this link must be always display Are you sure that this link must be always displayed? (i.e. it is not a part of some div about) Wenzhao (Colin) Zang 2017/04/10 20:37:35 I mimicked the original oobe_screen_reset.html str Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > Are you sure that this link must be always displayed? (i.e. it is not a part of > some div about) I mimicked the original oobe_screen_reset.html structure and the link is not part of a div. The hidden state of the link is controlled by the 'is-official-build' flag sent from reset_screen.cc. The flag value is actually never changed so the link is always hidden.
	27 hidden="[[isHelpLinkHidden_(uiState_, isOfficial_)]]"

	28 on-tap="onLearnMoreTap_" i18n-content="learnMore"></a>

	29 <div id="revert-promise-md" hidden="[[isRevertHidden_(uiState_)]]">

	30 <div i18n-content="resetRevertPromise"></div>

	31 <div id="reset-revert-spinner-md" class="throbber"></div>

	32 <div id="reset-revert-spinner-message-md"

	33 i18n-content="resetRevertSpinnerMessage">

	34 </div>

	35 </div>

	36 <img id="reset-illustration"

	37 src="chrome://theme/IDR_RESET_ILLUSTRATION"

	38 i18n-values="alt:resetScreenIllustrationTitle">

	39 </div>

	40 <div class="bottom-buttons flex layout horizontal">

	41 <div class="flex"></div>

	42 <oobe-text-button hidden="[[isCancelHidden_(uiState_)]]" border

	43 on-tap="onCancelTap_">

	44 <div i18n-content="cancelButton"></div>

	45 </oobe-text-button>

	46 <div class="vertical-margin"
	Alexander Alekseev 2017/04/08 10:36:18 Is it just a spacer? Could margin CSS attribute be Is it just a spacer? Could margin CSS attribute be used (for button)? Wenzhao (Colin) Zang 2017/04/10 20:37:35 Done. Sorry I ignored this much simpler method. Show quoted text On 2017/04/08 10:36:18, Alexander Alekseev wrote: > Is it just a spacer? Could margin CSS attribute be used (for button)? Done. Sorry I ignored this much simpler method.
	47 hidden="[[isCancelHidden_(uiState_)]]"></div>

	48 <oobe-text-button hidden="[[isRestartHidden_(uiState_)]]" inverse

	49 on-tap="onRestartTap_">
	Alexander Alekseev 2017/04/11 04:19:26 I believe you can also replace all the on.Tap_() I believe you can also replace all the on.Tap_() functions with userActed('action'); Wenzhao (Colin) Zang 2017/04/11 20:42:54 I tried something like <oobe-text-button on-tap=" Show quoted text On 2017/04/11 04:19:26, Alexander Alekseev wrote: > I believe you can also replace all the on.Tap_() functions with > userActed('action'); I tried something like <oobe-text-button on-tap="userActed" action="cancel-reset"> but it doesn't work, it seems that oobe-text-button does not have 'action' field as paper-button does. Alexander Alekseev* 2017/04/13 00:01:53 You're right, my comment was incorrect. Just ignor Show quoted text On 2017/04/11 20:42:54, Wenzhao (Colin) Zang wrote: > On 2017/04/11 04:19:26, Alexander Alekseev wrote: > > I believe you can also replace all the on.*Tap_() functions with > > userActed('action'); > > I tried something like > <oobe-text-button on-tap="userActed" action="cancel-reset"> > > but it doesn't work, it seems that oobe-text-button does not have 'action' field > as paper-button does. You're right, my comment was incorrect. Just ignore it.
	50 <div i18n-content="resetButtonRestart"></div>

	51 </oobe-text-button>

	52 <div class="vertical-margin"

	53 hidden="[[isRestartHidden_(uiState_)]]"></div>

	54 <oobe-text-button hidden="[[isDataWarningHidden_(uiState_)]]" inverse
	Alexander Alekseev 2017/04/11 04:19:26 Could you split this into two buttons? I see that Could you split this into two buttons? I see that they share the same action, but they look different to user, so let's make them separate. Wenzhao (Colin) Zang 2017/04/11 20:42:54 Done. Show quoted text On 2017/04/11 04:19:26, Alexander Alekseev wrote: > Could you split this into two buttons? > I see that they share the same action, but they look different to user, so let's > make them separate. Done.
	55 on-tap="onPowerwashTap_">

	56 <div i18n-content="resetButtonPowerwashAndRollback"

	57 hidden="[[isRollbackHidden_(uiState_)]]"></div>

	58 <div i18n-content="resetButtonPowerwash"

	59 hidden="[[isPowerwashHidden_(uiState_)]]"></div>

	60 </oobe-text-button>

	61 <div class="vertical-margin"

	62 hidden="[[isWarningDataHidden_(uiState_)]]"></div>

	63 </div>

	64 </oobe-dialog>

	65 </template>

	66 </dom-module>

OLD	NEW