Skip to content

GitLab

T
tangled
Commit dd751928 authored by dpapad's avatar dpapad Committed by Commit Bot
Browse files
Options

Fix a few UI bugs in chrome://signin-error. 

This is in preparation of migrating to Polymer3.

Specifically:
 - Adjust icon color in dark mode, previously same color was used
   in both light and dark mode.
 - Correctly hide "Learn more" link when it should be hidden.
   Previously was not correctly hidden because :empty CSS selector
   was always false.
 - Add missing icon on 3rd row of profile-blocking-error-message.
 - Remove unnecessary white space between #normal-error-message and
   #action-container.

In the process of fixing these bugs and paving the way for Polymer3:
 - Stop using "flattenhtml" to embed icons, instead use same icons
   from shared cr_elements/icons.html along with iron-icon.html
 - Extract most UI and logic in a <signin-error-app> element.
 - Leverage Polymer bindings to determine what to show/hide instead of
   :empty CSS selectors.

Bug: 1012533
Change-Id: Iaed8fdabbeb821f5c4d5b52f76510d477c55fd1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130809Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarRebekah Potter <rbpotter@chromium.org>
Commit-Queue: dpapad <dpapad@chromium.org>
Auto-Submit: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755510}
parent 0c540821
Hide whitespace changes
Inline Side-by-side
Showing with 228 additions and 140 deletions
chrome/browser/browser_resources.grd
View file @ dd751928
...@@ -190,8 +190,10 @@ ...@@ -190,8 +190,10 @@
<if expr="is_win or is_macosx or desktop_linux"> <if expr="is_win or is_macosx or desktop_linux">
<include name="IDR_SIGNIN_EMAIL_CONFIRMATION_HTML" file="resources\signin\signin_email_confirmation\signin_email_confirmation.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" /> <include name="IDR_SIGNIN_EMAIL_CONFIRMATION_HTML" file="resources\signin\signin_email_confirmation\signin_email_confirmation.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_SIGNIN_EMAIL_CONFIRMATION_APP_JS" file="${root_gen_dir}\chrome\browser\resources\signin\signin_email_confirmation\signin_email_confirmation_app.js" use_base_dir="false" preprocess="true" type="BINDATA" /> <include name="IDR_SIGNIN_EMAIL_CONFIRMATION_APP_JS" file="${root_gen_dir}\chrome\browser\resources\signin\signin_email_confirmation\signin_email_confirmation_app.js" use_base_dir="false" preprocess="true" type="BINDATA" />
<include name="IDR_SIGNIN_ERROR_HTML" file="resources\signin\signin_error\signin_error.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" /> <include name="IDR_SIGNIN_ERROR_HTML" file="resources\signin\signin_error\signin_error.html" type="BINDATA" />
<include name="IDR_SIGNIN_ERROR_JS" file="resources\signin\signin_error\signin_error.js" type="BINDATA" /> <include name="IDR_SIGNIN_ERROR_JS" file="resources\signin\signin_error\signin_error.js" type="BINDATA" />
<include name="IDR_SIGNIN_ERROR_APP_HTML" file="resources\signin\signin_error\signin_error_app.html" preprocess="true" type="BINDATA" />
<include name="IDR_SIGNIN_ERROR_APP_JS" file="resources\signin\signin_error\signin_error_app.js" type="BINDATA" />
</if> </if>
<include name="IDR_USB_ENUMERATION_OPTIONS_MOJOM_LITE_JS" file="${root_gen_dir}\services\device\public\mojom\usb_enumeration_options.mojom-lite.js" use_base_dir="false" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" /> <include name="IDR_USB_ENUMERATION_OPTIONS_MOJOM_LITE_JS" file="${root_gen_dir}\services\device\public\mojom\usb_enumeration_options.mojom-lite.js" use_base_dir="false" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" />
<include name="IDR_USB_DEVICE_MANAGER_CLIENT_MOJOM_LITE_JS" file="${root_gen_dir}\services\device\public\mojom\usb_manager_client.mojom-lite.js" use_base_dir="false" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" /> <include name="IDR_USB_DEVICE_MANAGER_CLIENT_MOJOM_LITE_JS" file="${root_gen_dir}\services\device\public\mojom\usb_manager_client.mojom-lite.js" use_base_dir="false" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" />
......
chrome/browser/resources/signin/signin_error/BUILD.gn
View file @ dd751928
...@@ -5,13 +5,19 @@ ...@@ -5,13 +5,19 @@
import("//third_party/closure_compiler/compile_js.gni") import("//third_party/closure_compiler/compile_js.gni")
js_type_check("closure_compile") { js_type_check("closure_compile") {
deps = [ ":signin_error" ] deps = [
":signin_error",
":signin_error_app",
]
} }
js_library("signin_error") { js_library("signin_error") {
deps = [ "//ui/webui/resources/js:cr" ]
}
js_library("signin_error_app") {
deps = [ deps = [
"//ui/webui/resources/js:cr",
"//ui/webui/resources/js:load_time_data", "//ui/webui/resources/js:load_time_data",
"//ui/webui/resources/js:util", "//ui/webui/resources/js:web_ui_listener_behavior",
] ]
} }
chrome/browser/resources/signin/signin_error/signin_error.html
View file @ dd751928
...@@ -2,107 +2,26 @@ ...@@ -2,107 +2,26 @@
<html dir="$i18n{textdirection}" lang="$i18n{language}"> <html dir="$i18n{textdirection}" lang="$i18n{language}">
<head> <head>
<meta charset="utf-8"> <meta charset="utf-8">
<link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
<link rel="import" href="chrome://resources/html/polymer.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html">
<link rel="import" href="signin_shared_old_css.html">
<link rel="stylesheet" href="chrome://resources/css/md_colors.css"> <link rel="stylesheet" href="chrome://resources/css/md_colors.css">
<link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">
<link rel="import" href="chrome://resources/html/cr.html">
<link rel="import" href="chrome://resources/html/load_time_data.html"> <link rel="import" href="chrome://resources/html/load_time_data.html">
<link rel="import" href="chrome://resources/html/util.html"> <script src="strings.js"></script>
<custom-style> <link rel="import" href="signin_error_app.html">
<style include="signin-dialog-shared"> <style>
@media (prefers-color-scheme: dark) { body {
html { margin: 0;
background: var(--md-background-color); }
}
} @media (prefers-color-scheme: dark) {
html {
.details { background: var(--md-background-color);
line-height: 20px; }
margin-bottom: 8px; }
margin-top: 16px; </style>
padding: 0 24px;
}
.details p {
margin-bottom: 0;
}
#closeButton {
margin-inline-start: 8px;
}
#normal-error-message p:empty,
#normal-error-message a:empty {
display: none;
}
#profile-blocking-error-message {
margin-top: 30px;
}
#profile-blocking-error-message p {
background-position: 0 3px;
background-repeat: no-repeat;
background-size: 20px;
line-height: 18px;
padding-inline-start: 35px;
}
html[dir=rtl] #profile-blocking-error-message p {
background-position: right 3px;
}
#profile-blocking-error-message p:empty {
display: none;
}
#profile-blocking-error-message p:nth-child(1) {
background-image:
url(../../../../../ui/webui/resources/images/business.svg);
}
#profile-blocking-error-message p:nth-child(2) {
background-image:
url(../../../../../ui/webui/resources/images/info.svg);
}
<if expr="is_macosx or is_linux">
#closeButton {
margin-inline-end: 8px;
margin-inline-start: 0;
}
</if>
</style>
</custom-style>
</head> </head>
<body> <body>
<div class="container"> <signin-error-app></signin-error-app>
<div class="top-title-bar">$i18n{signinErrorTitle}</div>
<div id="normal-error-message" class="details">
<p>$i18nRaw{signinErrorMessage}</p>
<a id="learnMoreLink" href="#">$i18nRaw{signinErrorLearnMore}</a>
</div>
<div id="profile-blocking-error-message" class="details">
<p>$i18n{profileBlockedMessage}</p>
<p>$i18n{profileBlockedAddPersonSuggestion}</p>
<p>$i18n{profileBlockedRemoveProfileSuggestion}</p>
</div>
<div class="action-container">
<cr-button class="action-button" id="switchButton">
$i18n{signinErrorSwitchLabel}
</cr-button>
<cr-button id="closeButton">
$i18n{signinErrorCloseLabel}
</cr-button>
<cr-button id="confirmButton" hidden>
$i18n{signinErrorOkLabel}
</cr-button>
</div>
</div>
</body> </body>
<link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">
<link rel="import" href="chrome://resources/html/cr.html">
<script src="signin_error.js"></script> <script src="signin_error.js"></script>
<script src="chrome://signin-error/strings.js"></script>
</html> </html>
chrome/browser/resources/signin/signin_error/signin_error.js
View file @ dd751928
...@@ -4,17 +4,7 @@ ...@@ -4,17 +4,7 @@
(function() { (function() {
function initialize() { function initialize() {
document.addEventListener('keydown', onKeyDown);
$('confirmButton').addEventListener('click', onConfirm);
$('closeButton').addEventListener('click', onConfirm);
$('switchButton').addEventListener('click', onSwitchToExistingProfile);
$('learnMoreLink').addEventListener('click', onLearnMore);
if (loadTimeData.getBoolean('isSystemProfile')) {
$('learnMoreLink').hidden = true;
}
cr.addWebUIListener('clear-focus', clearFocus); cr.addWebUIListener('clear-focus', clearFocus);
cr.addWebUIListener('switch-button-unavailable', removeSwitchButton);
// Prefer using |document.body.offsetHeight| instead of // Prefer using |document.body.offsetHeight| instead of
// |document.body.scrollHeight| as it returns the correct height of the // |document.body.scrollHeight| as it returns the correct height of the
...@@ -22,38 +12,9 @@ function initialize() { ...@@ -22,38 +12,9 @@ function initialize() {
chrome.send('initializedWithSize', [document.body.offsetHeight]); chrome.send('initializedWithSize', [document.body.offsetHeight]);
} }
function onKeyDown(e) {
// If the currently focused element isn't something that performs an action
// on "enter" being pressed and the user hits "enter", perform the default
// action of the dialog, which is "OK".
if (e.key == 'Enter' &&
!/^(A|CR-BUTTON)$/.test(document.activeElement.tagName)) {
$('confirmButton').click();
e.preventDefault();
}
}
function onConfirm(e) {
chrome.send('confirm');
}
function onSwitchToExistingProfile(e) {
chrome.send('switchToExistingProfile');
}
function onLearnMore(e) {
chrome.send('learnMore');
}
function clearFocus() { function clearFocus() {
document.activeElement.blur(); document.activeElement.blur();
} }
function removeSwitchButton() {
$('switchButton').hidden = true;
$('closeButton').hidden = true;
$('confirmButton').hidden = false;
}
document.addEventListener('DOMContentLoaded', initialize); document.addEventListener('DOMContentLoaded', initialize);
})(); })();
chrome/browser/resources/signin/signin_error/signin_error_app.html 0 → 100644
View file @ dd751928
<link rel="import" href="chrome://resources/html/polymer.html">
<link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
<link rel="import" href="chrome://resources/cr_elements/icons.html">
<link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
<link rel="import" href="chrome://resources/html/load_time_data.html">
<link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
<link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
<link rel="import" href="signin_shared_old_css.html">
<dom-module id="signin-error-app">
<template>
<style include="signin-dialog-shared">
.details {
line-height: 20px;
margin-bottom: 8px;
margin-top: 16px;
padding: 0 24px;
}
#normal-error-message > p {
margin-bottom: 0;
}
#closeButton {
margin-inline-start: 8px;
}
#profile-blocking-error-message {
margin-top: 30px;
}
#profile-blocking-error-message > div {
align-items: center;
display: flex;
margin-top: 1em;
}
iron-icon {
color: var(--cr-secondary-text-color);
flex-shrink: 0;
height: var(--cr-icon-size);
margin-inline-end: 16px;
width: var(--cr-icon-size);
}
<if expr="is_macosx or is_linux">
#closeButton {
margin-inline-end: 8px;
margin-inline-start: 0;
}
</if>
</style>
<div class="container">
<div class="top-title-bar">$i18n{signinErrorTitle}</div>
<div id="normal-error-message" class="details"
hidden="[[hideNormalError_]]">
<p>$i18nRaw{signinErrorMessage}</p>
<a id="learnMoreLink" href="#" on-click="onLearnMore_"
hidden="[[isSystemProfile_]]">
$i18nRaw{signinErrorLearnMore}
</a>
</div>
<div id="profile-blocking-error-message" class="details"
hidden="[[hideProfileBlockingErrors_.0]]">
<div hidden="[[hideProfileBlockingErrors_.1]]">
<iron-icon icon="cr:domain"></iron-icon>
<span>$i18n{profileBlockedMessage}</span>
</div>
<div hidden="[[hideProfileBlockingErrors_.2]]">
<iron-icon icon="cr:info"></iron-icon>
<span>$i18n{profileBlockedAddPersonSuggestion}</span>
</div>
<div hidden="[[hideProfileBlockingErrors_.3]]">
<iron-icon icon="cr:info"></iron-icon>
<span>$i18n{profileBlockedRemoveProfileSuggestion}</span>
</div>
</div>
<div class="action-container">
<cr-button class="action-button" id="switchButton"
hidden="[[switchButtonUnavailable_]]"
on-click="onSwitchToExistingProfile_">
$i18n{signinErrorSwitchLabel}
</cr-button>
<cr-button id="closeButton"
hidden="[[switchButtonUnavailable_]]"
on-click="onConfirm_">
$i18n{signinErrorCloseLabel}
</cr-button>
<cr-button id="confirmButton" hidden="[[!switchButtonUnavailable_]]"
on-click="onConfirm_">
$i18n{signinErrorOkLabel}
</cr-button>
</div>
</div>
</template>
<script src="signin_error_app.js"></script>
</dom-module>
chrome/browser/resources/signin/signin_error/signin_error_app.js 0 → 100644
View file @ dd751928
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
Polymer({
is: 'signin-error-app',
behaviors: [
WebUIListenerBehavior,
],
properties: {
/** @private {boolean} */
isSystemProfile_: {
type: Boolean,
value: () => loadTimeData.getBoolean('isSystemProfile'),
},
/** @private {boolean} */
switchButtonUnavailable_: {
type: Boolean,
value: false,
},
/** @private */
hideNormalError_: {
type: Boolean,
value: () => loadTimeData.getString('signinErrorMessage').length === 0,
},
/**
* An array of booleans indicating whether profile blocking messages should
* be hidden. Position 0 corresponds to the #profile-blocking-error-message
* container, and subsequent positions correspond to each of the 3 related
* messages respectively.
* @private {!Array<boolean>}
*/
hideProfileBlockingErrors_: {
type: Array,
value: function() {
const hide = [
'profileBlockedMessage',
'profileBlockedAddPersonSuggestion',
'profileBlockedRemoveProfileSuggestion',
].map(id => loadTimeData.getString(id).length === 0);
// Hide the container itself if all of each children are also hidden.
hide.unshift(hide.every(hideEntry => hideEntry));
return hide;
},
},
},
/** @private {?function(!Event)} */
boundKeyDownHandler_: null,
/** @override */
attached() {
this.boundKeyDownHandler_ = this.onKeyDown_.bind(this);
document.addEventListener('keydown', this.boundKeyDownHandler_);
this.addWebUIListener('switch-button-unavailable', () => {
this.switchButtonUnavailable_ = true;
});
},
/** @override */
detached() {
document.removeEventListener('keydown', this.boundKeyDownHandler_);
this.boundKeyDownHandler_ = null;
},
/** @private */
onConfirm_() {
chrome.send('confirm');
},
/** @private */
onSwitchToExistingProfile_() {
chrome.send('switchToExistingProfile');
},
/** @private */
onLearnMore_() {
chrome.send('learnMore');
},
/**
* @param {!Event} e
* @private
*/
onKeyDown_(e) {
if (e.key == 'Enter' &&
!/^(A|CR-BUTTON)$/.test(e.composedPath()[0].tagName)) {
this.onConfirm_();
e.preventDefault();
}
},
});
chrome/browser/ui/webui/signin/signin_error_ui.cc
View file @ dd751928
...@@ -68,6 +68,8 @@ void SigninErrorUI::Initialize(Browser* browser, bool is_system_profile) { ...@@ -68,6 +68,8 @@ void SigninErrorUI::Initialize(Browser* browser, bool is_system_profile) {
content::WebUIDataSource::Create(chrome::kChromeUISigninErrorHost); content::WebUIDataSource::Create(chrome::kChromeUISigninErrorHost);
source->UseStringsJs(); source->UseStringsJs();
source->SetDefaultResource(IDR_SIGNIN_ERROR_HTML); source->SetDefaultResource(IDR_SIGNIN_ERROR_HTML);
source->AddResourcePath("signin_error_app.html", IDR_SIGNIN_ERROR_APP_HTML);
source->AddResourcePath("signin_error_app.js", IDR_SIGNIN_ERROR_APP_JS);
source->AddResourcePath("signin_error.js", IDR_SIGNIN_ERROR_JS); source->AddResourcePath("signin_error.js", IDR_SIGNIN_ERROR_JS);
source->AddResourcePath("signin_shared_old_css.html", source->AddResourcePath("signin_shared_old_css.html",
IDR_SIGNIN_SHARED_OLD_CSS_HTML); IDR_SIGNIN_SHARED_OLD_CSS_HTML);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment