Commit dc2a34a9 authored by Daniel Clark's avatar Daniel Clark Committed by Commit Bot

Fix escape key behavior for embedded and standalone month pickers

This change fixes a couple of related issues about keyboard input
the standalone month picker control and the month switcher menu embedded
in other calendar picker controls.

- The standalone month picker didn't close when the user hits Escape.
The code for this was just missing (the month picker was built by reusing
the YearListView for embedded month switcher menus embedded in other
calendar popups, for which Escape is handled by the outer control). I've
added the code to handle Escape.

- I noticed that the escape was also not working to dismiss the month
switcher menu when opened in a datetime-local.  One problem with fixing
this was was that the top-level DateTimeLocalPicker had no way to
distinguish whether a EventTypeYearListViewDidHide was triggered by an
Escape or Enter, where Escape would mean that the display should go
back to the previously selected month.  I reworked how the
YearListView dispatches events to make this work; it now sends
EventTypeYearListViewDidSelectMonth to indicate that a month was chosen
and that the control should be dismissed, sends
EventTypeYearListViewDidHide to indicate that the month switcher should
be dismissed without changes, and sends nothing when the user
arrow-key-navigates between months.  One side-effect is that the
month displayed on the month switcher button no longer updates as the user
navigates between months, but this is consistent with the no-live-update
behavior of our standalone month/week/date pickers.  If we switch those
to live-update, we should consider switching this behavior
correspondingly.  These changes are gated behind isFormControlsRefresh
so that the old picker keeps the old behavior.

- While working on the above I also noticed that clicking the body of a
datetime-local or or on the month switcher or previous/next month
buttons would cause the popup's value to be pushed to the in-page control.
I fixed this by getting rid of the approach where parts of the calendar
control suppress events so that they are not seen by the top level
control, and instead equip the top-level DateTimePicker class to filter
the keyboard and click events it receives so that it only acts on the
ones with the relevant event.targets (time and date cells and the Today
button).

Tests are included for all of the above.

Bug: 1026489
Change-Id: I0e66dae98d891c79ea7e1fe2821c948eb9aef499
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003431Reviewed-by: default avatarIonel Popescu <iopopesc@microsoft.com>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#733053}
parent 7b00a635
......@@ -2505,8 +2505,10 @@ YearListView.prototype.onClick = function(event) {
// Always start with first month when changing the year.
const month = new Month(year, 0);
this.highlightMonth(month);
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this, month);
if (!global.params.isFormControlsRefreshEnabled) {
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this, month);
}
this.scrollView.scrollTo(this.selectedRow * YearListCell.GetHeight(), true);
} else {
var monthButton = enclosingNodeOrSelfWithClass(
......@@ -2517,7 +2519,9 @@ YearListView.prototype.onClick = function(event) {
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this,
new Month(year, month));
this.hide();
if (!global.params.isFormControlsRefreshEnabled) {
this.hide();
}
}
};
......@@ -2795,8 +2799,11 @@ YearListView.prototype._moveHighlightTo = function(month) {
this.highlightMonth(month);
this.select(this.highlightedMonth.year - 1);
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this, month);
if (!global.params.isFormControlsRefreshEnabled) {
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this, month);
}
this.scrollView.scrollTo(this.selectedRow * YearListCell.GetHeight(), true);
return true;
};
......@@ -2831,6 +2838,11 @@ YearListView.prototype.onKeyDown = function(event) {
this.dispatchEvent(
YearListView.EventTypeYearListViewDidSelectMonth, this,
this.highlightedMonth);
if (!global.params.isFormControlsRefreshEnabled) {
this.hide();
}
eventHandled = true;
} else if (key == 'Escape' && global.params.isFormControlsRefreshEnabled) {
this.hide();
eventHandled = true;
}
......@@ -2950,7 +2962,6 @@ function MonthPopupButton(maxWidth) {
this.element.style.maxWidth = maxWidth + 'px';
this.element.addEventListener('click', this.onClick, false);
this.element.addEventListener('keydown', this.onKeyDown, false);
}
MonthPopupButton.prototype = Object.create(View.prototype);
......@@ -2993,20 +3004,6 @@ MonthPopupButton.prototype.onClick = function(event) {
this.dispatchEvent(MonthPopupButton.EventTypeButtonClick, this);
};
/**
* @param {?Event} event
*/
MonthPopupButton.prototype.onKeyDown = function(event) {
if (event.key === 'Enter') {
// Prevent an Enter keypress on the month selector from submitting the
// popup. The click handler will also run for a keypress even though this
// event is suppressed, so the month popup will still open from the
// Enter key.
event.stopPropagation();
event.preventDefault();
}
};
/**
* @constructor
* @extends View
......@@ -3032,7 +3029,6 @@ function CalendarNavigationButton() {
*/
this._timer = null;
this.element.addEventListener('click', this.onClick, false);
this.element.addEventListener('keydown', this.onKeyDown, false);
this.element.addEventListener('mousedown', this.onMouseDown, false);
this.element.addEventListener('touchstart', this.onTouchStart, false);
};
......@@ -3062,20 +3058,6 @@ CalendarNavigationButton.prototype.onClick = function(event) {
this.dispatchEvent(CalendarNavigationButton.EventTypeButtonClick, this);
};
/**
* @param {?Event} event
*/
CalendarNavigationButton.prototype.onKeyDown = function(event) {
if (event.key === 'Enter') {
// Prevent an Enter keypress on the previous/next month and Today buttons
// from submitting the popup. The click handler will also run for a
// keypress even though this event is suppressed, so the action for the
// button will still be triggered from the Enter key.
event.stopPropagation();
event.preventDefault();
}
};
/**
* @param {?Event} event
*/
......@@ -3179,7 +3161,6 @@ function CalendarHeaderView(calendarPicker) {
*/
this._todayButton = new CalendarNavigationButton();
this._todayButton.attachTo(this);
this._todayButton.isTodayButton = true;
this._todayButton.on(
CalendarNavigationButton.EventTypeButtonClick,
this.onNavigationButtonClick);
......@@ -3677,7 +3658,6 @@ function CalendarTableView(calendarPicker) {
* @const
*/
var todayButton = new CalendarNavigationButton();
todayButton.isTodayButton = true;
todayButton.attachTo(this);
todayButton.on(
CalendarNavigationButton.EventTypeButtonClick, this.onTodayButtonClick);
......@@ -4146,6 +4126,7 @@ CalendarPicker.prototype.onYearListViewDidHide = function(sender) {
this.calendarHeaderView.setDisabled(false);
if (global.params.isFormControlsRefreshEnabled) {
this.calendarTableView.element.style.visibility = 'visible';
this.calendarTableView.element.focus();
} else {
this.adjustHeight();
}
......@@ -4158,6 +4139,9 @@ CalendarPicker.prototype.onYearListViewDidHide = function(sender) {
CalendarPicker.prototype.onYearListViewDidSelectMonth = function(
sender, month) {
this.setCurrentMonth(month, CalendarPicker.NavigationBehavior.None);
if (global.params.isFormControlsRefreshEnabled) {
this.onYearListViewDidHide();
}
};
/**
......
......@@ -46,7 +46,14 @@ class DateTimeLocalPicker extends HTMLElement {
onKeyDown_ = (event) => {
switch (event.key) {
case 'Enter':
window.pagePopupController.setValueAndClosePopup(0, this.selectedValue);
// Submit the popup for an Enter keypress except when the user is
// hitting Enter to activate the month switcher button, Today button,
// or previous/next month arrows.
if (!event.target.matches(
'.calendar-navigation-button, .month-popup-button')) {
window.pagePopupController.setValueAndClosePopup(
0, this.selectedValue);
}
break;
case 'Escape':
if (this.selectedValue === this.initialSelectedValue_) {
......@@ -68,7 +75,8 @@ class DateTimeLocalPicker extends HTMLElement {
};
onClick_ = (event) => {
if (this.hasSelectedDate) {
if (event.target.matches('.day-cell, .time-cell, .today-button-refresh') &&
this.hasSelectedDate) {
window.pagePopupController.setValue(this.selectedValue);
}
};
......
......@@ -90,14 +90,13 @@ class MonthPicker extends HTMLElement {
};
onYearListViewDidHide_ = (sender) => {
const selectedValue = this.selectedMonth_.toString();
window.setTimeout(function() {
window.pagePopupController.setValueAndClosePopup(0, selectedValue);
}, 100);
window.pagePopupController.closePopup();
};
onYearListViewDidSelectMonth_ = (sender, month) => {
this.selectedMonth_ = month;
const selectedValue = this.selectedMonth_.toString();
window.pagePopupController.setValueAndClosePopup(0, selectedValue);
};
initializeTodayButton_ = () => {
......@@ -116,9 +115,7 @@ class MonthPicker extends HTMLElement {
onTodayButtonClick_ = (sender) => {
const selectedValue = Month.createFromToday().toString();
window.setTimeout(function() {
window.pagePopupController.setValueAndClosePopup(0, selectedValue);
}, 100);
window.pagePopupController.setValueAndClosePopup(0, selectedValue);
};
onWindowResize_ = (event) => {
......
......@@ -116,7 +116,10 @@ function highlightedMonthButton() {
function skipAnimationAndGetPositionOfMonthPopupButton() {
skipAnimation();
var buttonElement = popupWindow.global.picker.calendarHeaderView.monthPopupButton.element;
const calendarHeaderView = popupWindow.global.picker.datePicker ?
popupWindow.global.picker.datePicker.calendarHeaderView :
popupWindow.global.picker.calendarHeaderView;
var buttonElement = calendarHeaderView.monthPopupButton.element;
var offset = cumulativeOffset(buttonElement);
return {x: offset[0] + buttonElement.offsetWidth / 2, y: offset[1] + buttonElement.offsetHeight / 2};
}
......@@ -132,6 +135,53 @@ function clickMonthPopupButton() {
eventSender.mouseUp();
}
function skipAnimationAndGetPositionOfTodayButton() {
skipAnimation();
const calendarTableView = popupWindow.global.picker.datePicker ?
popupWindow.global.picker.datePicker.calendarTableView :
popupWindow.global.picker.calendarTableView;
var buttonElement =
calendarTableView.element.querySelector('.today-button-refresh');
var offset = cumulativeOffset(buttonElement);
return {
x: offset[0] + buttonElement.offsetWidth / 2,
y: offset[1] + buttonElement.offsetHeight / 2
};
}
function hoverOverTodayButton() {
var position = skipAnimationAndGetPositionOfTodayButton();
eventSender.mouseMoveTo(position.x, position.y);
}
function clickTodayButton() {
hoverOverTodayButton();
eventSender.mouseDown();
eventSender.mouseUp();
}
function skipAnimationAndGetPositionOfThisMonthButton() {
skipAnimation();
const button =
popupWindow.global.picker.querySelector('.today-button-refresh');
var offset = cumulativeOffset(button);
return {
x: offset[0] + button.offsetWidth / 2,
y: offset[1] + button.offsetHeight / 2
};
}
function hoverOverThisMonthButton() {
var position = skipAnimationAndGetPositionOfThisMonthButton();
eventSender.mouseMoveTo(position.x, position.y);
}
function clickThisMonthButton() {
hoverOverThisMonthButton();
eventSender.mouseDown();
eventSender.mouseUp();
}
function clickYearListCell(year) {
skipAnimation();
var row = year - 1;
......
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<script src="../../../fast/forms/resources/picker-common.js"></script>
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="date" id="date-0" value="2019-02-14">
<input type="date" id="date-1" value="2019-02-14">
<script>
promise_test(() => {
let dateElement = document.getElementById('date-0');
return openPickerWithPromise(dateElement)
.then(() => {
// Make the picker dismiss synchronously so we don't need to insert
// an artificial delay in the test
internals.pagePopupWindow.CalendarPicker.commitDelayMs = 0;
clickMonthPopupButton();
eventSender.keyDown('ArrowRight');
eventSender.keyDown('ArrowDown');
eventSender.keyDown('Enter');
clickDayCellAt(2, 3);
assert_equals(dateElement.value, "2019-07-23", "Month chooser button should have changed month");
assert_equals(internals.pagePopupWindow, null, "Clicking a date should dismiss popup.");
});
}, "Date picker: Month chooser should allow user to chooser month");
promise_test(() => {
let dateElement = document.getElementById('date-1');
return openPickerWithPromise(dateElement)
.then(() => {
// Make the picker dismiss synchronously so we don't need to insert
// an artificial delay in the test
internals.pagePopupWindow.CalendarPicker.commitDelayMs = 0;
clickMonthPopupButton();
eventSender.keyDown('ArrowRight');
eventSender.keyDown('ArrowDown');
eventSender.keyDown('Escape');
clickDayCellAt(2, 3);
assert_equals(dateElement.value, "2019-02-19", "Escape key should have dismissed month chooser without changing month ");
});
}, "Date picker: Month chooser should dismiss on Escape key");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<script src="../../../fast/forms/resources/picker-common.js"></script>
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="date" id="date" value="2019-02-14">
<script>
promise_test(() => {
let dateElement = document.getElementById('date');
return openPickerWithPromise(dateElement)
.then(() => {
// Make the picker dismiss synchronously so we don't need to insert
// an artificial delay in the test
internals.pagePopupWindow.CalendarPicker.commitDelayMs = 0;
clickTodayButton();
let splitDate = dateElement.value.split('-');
let actualTodayDateString = new Date(splitDate[0], splitDate[1] - 1, [splitDate[2]]).toDateString();
let expectedTodayDateString = new Date().toDateString();
assert_equals(actualTodayDateString, expectedTodayDateString, "Today button should have updated in-page control to today's date");
assert_equals(internals.pagePopupWindow, null, "Click on Today button should close popup.");
});
}, "Date picker: Today button should select today's date");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<script src="../../../fast/forms/resources/picker-common.js"></script>
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="datetime-local" id="datetime-local-0" value="2019-02-14T13:02">
<input type="datetime-local" id="datetime-local-1" value="2019-02-14T13:02">
<input type="datetime-local" id="datetime-local-2">
<script>
promise_test(() => {
let dateTimeElement = document.getElementById('datetime-local-0');
return openPickerWithPromise(dateTimeElement)
.then(() => {
clickMonthPopupButton();
eventSender.keyDown('ArrowRight');
eventSender.keyDown('ArrowDown');
eventSender.keyDown('Enter');
clickDayCellAt(2, 3);
assert_equals(dateTimeElement.value, "2019-07-23T13:02", "Month chooser button should have changed month");
eventSender.keyDown('Enter');
assert_equals(internals.pagePopupWindow, null, "Enter key should dismiss popup.");
});
}, "Datetimelocal picker: Month chooser should allow user to chooser month");
promise_test(() => {
let dateTimeElement = document.getElementById('datetime-local-1');
return openPickerWithPromise(dateTimeElement)
.then(() => {
clickMonthPopupButton();
eventSender.keyDown('ArrowRight');
eventSender.keyDown('ArrowDown');
eventSender.keyDown('Escape');
clickDayCellAt(2, 3);
assert_equals(dateTimeElement.value, "2019-02-19T13:02", "Escape key should have dismissed month chooser without changing month.");
eventSender.keyDown('Enter');
assert_equals(internals.pagePopupWindow, null, "Enter key should dismiss popup.");
});
}, "Datetimelocal picker: Month chooser should dismiss on Escape key");
promise_test(() => {
let dateTimeElement = document.getElementById('datetime-local-2');
return openPickerWithPromise(dateTimeElement)
.then(() => {
clickMonthPopupButton();
eventSender.keyDown('ArrowRight');
eventSender.keyDown('ArrowDown');
eventSender.keyDown('Enter');
assert_equals(dateTimeElement.value, "", "Opening month switcher should not push value to in-page control.");
eventSender.keyDown('Escape');
assert_equals(internals.pagePopupWindow, null, "Enter key should dismiss popup.");
});
}, "Datetimelocal picker: Opening month chooser should not push value to in-page control");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<script src="../../../fast/forms/resources/picker-common.js"></script>
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="datetime-local" id="datetime-local" value="2019-02-14T13:02">
<script>
promise_test(() => {
let dateTimeElement = document.getElementById('datetime-local');
return openPickerWithPromise(dateTimeElement)
.then(() => {
clickTodayButton();
let actualTodayDateString = new Date(dateTimeElement.value).toDateString();
let expectedTodayDateString = new Date().toDateString();
assert_equals(actualTodayDateString, expectedTodayDateString, "Today button should have updated in-page control to today's date");
assert_not_equals(internals.pagePopupWindow, null, "Click on Today button should not close popup.");
eventSender.keyDown('Escape');
eventSender.keyDown('Escape');
assert_equals(internals.pagePopupWindow, null, "Double-Escape should dismiss popup.");
});
}, "Datetimelocal picker: Today button should select today's date");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src='../../../resources/testharness.js'></script>
<script src='../../../resources/testharnessreport.js'></script>
<script src='../../../fast/forms/resources/picker-common.js'></script>
</head>
<body>
<input type='month' id='month'>
<script>
'use strict';
promise_test(() => {
let monthControl = document.getElementById('month');
return openPickerWithPromise(monthControl)
.then(() => {
internals.pagePopupWindow.focus();
const popupDocument = internals.pagePopupWindow.document;
eventSender.keyDown('ArrowRight');
assert_not_equals(internals.pagePopupWindow, null, "Popup should still be open");
assert_equals(monthControl.value, "", "Arrow key navigation in month popup should not change value");
eventSender.keyDown('Escape');
assert_equals(internals.pagePopupWindow, null, "Escape key should close month popup.");
assert_equals(monthControl.value, "", "Closing month popup with escape key should not commit value");
});
}, "Month picker: Pressing Escape key closes popup without changing value");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../../fast/forms/resources/common.js"></script>
<script src="../../../fast/forms/resources/picker-common.js"></script>
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="month" id="month" value="2019-02">
<script>
promise_test(() => {
let monthElement = document.getElementById('month');
return openPickerWithPromise(monthElement)
.then(() => {
clickThisMonthButton();
let splitDate = monthElement.value.split('-');
let actualTodayDateString = new Date(splitDate[0], splitDate[1] - 1).toDateString();
let today = new Date();
today.setDate(1);
let expectedTodayDateString = today.toDateString();
assert_equals(actualTodayDateString, expectedTodayDateString, "'This month button should have updated in-page control to this month");
assert_equals(internals.pagePopupWindow, null, "Click on 'This month button should close popup.");
});
}, "Date picker: 'This month' button should select current month");
</script>
</body>
</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