Commit 0a076d80 authored by Daniel Clark's avatar Daniel Clark Committed by Commit Bot

Reland "Color control popup UI improvements"

Fixed the following issues that led to the revert:
- Added missing mac image baseline updates
- Some tests I added/extended were timing out in a linux MSAN run.  I noticed that the color
picker is slow to open in even a normal debug build, so I suspect that these are legit timeouts
where the tests are just taking too long on the MSAN build.  I've split the tests up to avoid
this (color-picker-events-*, color-picker-escape-cancellation-*).

Original change description follows:

This CL updates the <input type="color"> popup UI in three related aspects per design feedback:

1) The in-page control now updates immediately as the selected color is changed in the popup,
instead of waiting for the popup to be dismissed.  The page can watch for these 'live' changes
via the input event.
2) The UI is simplified by removal of the submit and cancel buttons.  The popup is now
dismissed only by clicking outside of it or with the Enter or Escape keys.
3) A 'double-escape' model is introduced in which if a user has opened the popup and changed the
value, hitting Escape once will reset the value back to what it was when the popup was opened.
Hitting Escape once more, when the popup value matches the value when opened, will dismiss the
popup.

Bug: 1001571
Change-Id: I1369851208c3da0231b6c1d6fb218b9c13e00ca5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992598
Commit-Queue: Dan Clark <daniec@microsoft.com>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729968}
parent ec6d3966
...@@ -16,12 +16,12 @@ color-picker { ...@@ -16,12 +16,12 @@ color-picker {
background: #FFFFFF; background: #FFFFFF;
display: flex; display: flex;
flex-direction: column; flex-direction: column;
height: 304px; height: 250px;
width: 232px; width: 232px;
} }
visual-color-picker { visual-color-picker {
height: 59%; height: 71.5%;
min-height: 0; min-height: 0;
} }
...@@ -167,31 +167,6 @@ channel-label { ...@@ -167,31 +167,6 @@ channel-label {
width: 172px; width: 172px;
} }
submission-controls {
align-items: center;
border-top: 1px solid #CECECE;
display: flex;
flex-direction: row;
height: 13%;
min-height: 0;
}
#submission-controls-padding {
height: 100%;
width: 84%;
}
submission-button {
padding: 3%;
text-align: center;
width: 8%;
}
submission-button:hover {
background-color: #F3F3F3;
border-radius: 2px;
}
@media (forced-colors: active) { @media (forced-colors: active) {
color-viewer { color-viewer {
forced-color-adjust: none; forced-color-adjust: none;
......
...@@ -425,14 +425,11 @@ class ColorPicker extends HTMLElement { ...@@ -425,14 +425,11 @@ class ColorPicker extends HTMLElement {
super(); super();
this.selectedColor_ = initialColor; this.selectedColor_ = initialColor;
this.colorWhenOpened_ = initialColor;
this.visualColorPicker_ = new VisualColorPicker(initialColor); this.visualColorPicker_ = new VisualColorPicker(initialColor);
this.manualColorPicker_ = new ManualColorPicker(initialColor); this.manualColorPicker_ = new ManualColorPicker(initialColor);
this.submissionControls_ = new SubmissionControls( this.append(this.visualColorPicker_, this.manualColorPicker_);
this.onSubmitButtonClick_, this.onCancelButtonClick_);
this.append(
this.visualColorPicker_, this.manualColorPicker_,
this.submissionControls_);
this.visualColorPicker_.addEventListener( this.visualColorPicker_.addEventListener(
'visual-color-picker-initialized', this.initializeListeners_); 'visual-color-picker-initialized', this.initializeListeners_);
...@@ -480,6 +477,9 @@ class ColorPicker extends HTMLElement { ...@@ -480,6 +477,9 @@ class ColorPicker extends HTMLElement {
this.processingManualColorChange_ = true; this.processingManualColorChange_ = true;
this.visualColorPicker_.color = newColor; this.visualColorPicker_.color = newColor;
this.processingManualColorChange_ = false; this.processingManualColorChange_ = false;
const selectedValue = newColor.asHex();
window.pagePopupController.setValue(selectedValue);
} }
} }
...@@ -492,6 +492,9 @@ class ColorPicker extends HTMLElement { ...@@ -492,6 +492,9 @@ class ColorPicker extends HTMLElement {
if (!this.processingManualColorChange_) { if (!this.processingManualColorChange_) {
this.selectedColor = newColor; this.selectedColor = newColor;
this.manualColorPicker_.color = newColor; this.manualColorPicker_.color = newColor;
const selectedValue = newColor.asHex();
window.pagePopupController.setValue(selectedValue);
} else { } else {
// We are making a visual color change in response to a manual color // We are making a visual color change in response to a manual color
// change. So we do not overwrite the manually specified values and do // change. So we do not overwrite the manually specified values and do
...@@ -506,10 +509,16 @@ class ColorPicker extends HTMLElement { ...@@ -506,10 +509,16 @@ class ColorPicker extends HTMLElement {
onKeyDown_ = (event) => { onKeyDown_ = (event) => {
switch(event.key) { switch(event.key) {
case 'Enter': case 'Enter':
this.submissionControls_.submitButton.click(); window.pagePopupController.closePopup();
break; break;
case 'Escape': case 'Escape':
this.submissionControls_.cancelButton.click(); if (this.selectedColor.equals(this.colorWhenOpened_)) {
window.pagePopupController.closePopup();
} else {
this.manualColorPicker_.dispatchEvent(new CustomEvent(
'manual-color-change',
{bubbles: true, detail: {color: this.colorWhenOpened_}}));
}
break; break;
case 'Tab': case 'Tab':
event.preventDefault(); event.preventDefault();
...@@ -539,21 +548,6 @@ class ColorPicker extends HTMLElement { ...@@ -539,21 +548,6 @@ class ColorPicker extends HTMLElement {
'color-value-container:not(.hidden-color-value-container) > input,' + 'color-value-container:not(.hidden-color-value-container) > input,' +
'[tabindex]:not([tabindex=\'-1\'])')); '[tabindex]:not([tabindex=\'-1\'])'));
} }
static get COMMIT_DELAY_MS() {
return 100;
}
onSubmitButtonClick_ = () => {
const selectedValue = this.selectedColor_.asHex();
window.setTimeout(function() {
window.pagePopupController.setValueAndClosePopup(0, selectedValue);
}, ColorPicker.COMMIT_DELAY_MS);
};
onCancelButtonClick_ = () => {
window.pagePopupController.closePopup();
};
} }
window.customElements.define('color-picker', ColorPicker); window.customElements.define('color-picker', ColorPicker);
...@@ -1873,65 +1867,3 @@ class ChannelLabel extends HTMLElement { ...@@ -1873,65 +1867,3 @@ class ChannelLabel extends HTMLElement {
} }
} }
window.customElements.define('channel-label', ChannelLabel); window.customElements.define('channel-label', ChannelLabel);
/**
* SubmissionControls: Provides functionality to submit or discard a change.
*/
class SubmissionControls extends HTMLElement {
/**
* @param {function} submitCallback executed if the submit button is clicked
* @param {function} cancelCallback executed if the cancel button is clicked
*/
constructor(submitCallback, cancelCallback) {
super();
const padding = document.createElement('span');
padding.setAttribute('id', 'submission-controls-padding');
this.append(padding);
this.submitButton_ = new SubmissionButton(
submitCallback,
'<svg width="14" height="10" viewBox="0 0 14 10" fill="none" ' +
'xmlns="http://www.w3.org/2000/svg"><path d="M13.3516 ' +
'1.35156L5 9.71094L0.648438 5.35156L1.35156 4.64844L5 ' +
'8.28906L12.6484 0.648438L13.3516 1.35156Z" fill="WindowText"/></svg>');
this.cancelButton_ = new SubmissionButton(
cancelCallback,
'<svg width="14" height="14" viewBox="0 0 14 14" fill="none" ' +
'xmlns="http://www.w3.org/2000/svg"><path d="M7.71094 7L13.1016 ' +
'12.3984L12.3984 13.1016L7 7.71094L1.60156 13.1016L0.898438 ' +
'12.3984L6.28906 7L0.898438 1.60156L1.60156 0.898438L7 ' +
'6.28906L12.3984 0.898438L13.1016 1.60156L7.71094 7Z" ' +
'fill="WindowText"/></svg>');
this.append(this.submitButton_, this.cancelButton_);
}
get submitButton() {
return this.submitButton_;
}
get cancelButton() {
return this.cancelButton_;
}
}
window.customElements.define('submission-controls', SubmissionControls);
/**
* SubmissionButton: Button with a custom look that can be clicked for
* a submission action.
*/
class SubmissionButton extends HTMLElement {
/**
* @param {function} clickCallback executed when the button is clicked
* @param {string} htmlString custom look for the button
*/
constructor(clickCallback, htmlString) {
super();
this.setAttribute('tabIndex', '0');
this.innerHTML = htmlString;
this.addEventListener('click', clickCallback);
}
}
window.customElements.define('submission-button', SubmissionButton);
...@@ -11,8 +11,8 @@ function popupOpenCallbackWrapper() { ...@@ -11,8 +11,8 @@ function popupOpenCallbackWrapper() {
setTimeout(popupOpenCallback, 20); setTimeout(popupOpenCallback, 20);
} }
function waitUntilClosing(callback, customDelay) { function waitUntilClosing(callback) {
setTimeout(callback, (customDelay !== undefined) ? customDelay : 1); setTimeout(callback, 1);
} }
function rootWindow() { function rootWindow() {
...@@ -34,6 +34,34 @@ function rootWindow() { ...@@ -34,6 +34,34 @@ function rootWindow() {
// - INPUT color with DATALIST // - INPUT color with DATALIST
// - INPUT date/datetime-local/month/week // - INPUT date/datetime-local/month/week
function openPicker(element, callback, errorCallback) { function openPicker(element, callback, errorCallback) {
popupWindow = openPickerHelper(element);
if (typeof callback === "function" && popupWindow)
setPopupOpenCallback(callback);
else if (typeof errorCallback === "function" && !popupWindow)
errorCallback();
}
// openPickerWithPromise opens a picker UI for the following types:
// - menulist SELECT
// - INPUT color
// - INPUT date/datetime-local/month/week
//
// Returns a Promise that resolves when the popup has been opened.
function openPickerWithPromise(element) {
return new Promise(function(resolve, reject) {
popupWindow = openPickerHelper(element);
if (popupWindow) {
popupWindow.addEventListener("didOpenPicker", resolve, false);
} else {
reject();
}
});
}
// Helper function for openPicker and openPickerWithPromise.
// Performs the keystrokes that will cause the picker to open,
// and returns the popup window, or null.
function openPickerHelper(element) {
element.offsetTop; // Force to lay out element.offsetTop; // Force to lay out
element.focus(); element.focus();
if (element.tagName === "SELECT") { if (element.tagName === "SELECT") {
...@@ -45,11 +73,7 @@ function openPicker(element, callback, errorCallback) { ...@@ -45,11 +73,7 @@ function openPicker(element, callback, errorCallback) {
eventSender.keyDown("ArrowDown", ["altKey"]); eventSender.keyDown("ArrowDown", ["altKey"]);
} }
} }
popupWindow = internals.pagePopupWindow; return internals.pagePopupWindow;
if (typeof callback === "function" && popupWindow)
setPopupOpenCallback(callback);
else if (typeof errorCallback === "function" && !popupWindow)
errorCallback();
} }
function clickToOpenPicker(x, y, callback, errorCallback) { function clickToOpenPicker(x, y, callback, errorCallback) {
......
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