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

Fix Enter key usage with 'This month' button in month input popup

Fix an issue where focusing the 'This month' button in a month input
popup and pressing 'Enter' would close the popup but not switch
to the current month.

The problem was that the MonthPicker.onKeyDown_ handler was receiving
the event first and submitting the popup with the current value (which
was the correct behavior for an 'Enter' keypress anywhere else in the
popup).  With this fix, the onKeyDown_ handler skips the event if
the target was the 'This month' button so that
MonthPicker.onTodayButtonClick_ can handle it correctly.

An alternative solution that I explored was to move the element for
the 'This month' button under the YearListView so that the button's
event handler would be triggered before the keydown handler on the
MonthPicker, but this caused problems with the popup layout while
leaving other issues with the event ordering, so I chose to go with
the simpler solution found in this CL.

Bug: 1054469
Change-Id: I4f32b6ca8b71b7e700e63c8e2d45429df74dce27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067718Reviewed-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@{#744080}
parent c84ff237
......@@ -92,8 +92,7 @@ class MonthPicker extends HTMLElement {
this.todayButton_.element.classList.add(MonthPicker.ClassNameTodayButton);
const monthContainingToday = Month.createFromToday();
this.todayButton_.setDisabled(
monthContainingToday < this.minimumMonth_ ||
monthContainingToday > this.maximumMonth_);
!this.yearListView_.isValid(monthContainingToday));
this.todayButton_.on(
CalendarNavigationButton.EventTypeButtonClick,
this.onTodayButtonClick_);
......@@ -107,7 +106,6 @@ class MonthPicker extends HTMLElement {
onKeyDown_ = (event) => {
switch (event.key) {
case 't':
case 'Enter':
if (this.selectedMonth) {
window.pagePopupController.setValueAndClosePopup(
0, this.selectedMonth.toString());
......@@ -115,6 +113,18 @@ class MonthPicker extends HTMLElement {
window.pagePopupController.closePopup();
}
break;
case 'Enter':
// Don't do anything here if user has hit Enter on 'This month'
// button. We'll handle that in this.onTodayButtonClick_.
if (!event.target.matches('.calendar-navigation-button')) {
if (this.selectedMonth) {
window.pagePopupController.setValueAndClosePopup(
0, this.selectedMonth.toString());
} else {
window.pagePopupController.closePopup();
}
}
break;
case 'Escape':
if (!this.selectedMonth ||
(this.selectedMonth.equals(this.initialSelectedMonth))) {
......
......@@ -8,11 +8,13 @@
<script src="../../../fast/forms/calendar-picker/resources/calendar-picker-common.js"></script>
</head>
<body>
<input type="month" id="month" value="2019-02">
<input type="month" id="month0" value="2019-02">
<input type="month" id="month1" value="2019-02">
<input type="month" id="month2" value="2019-02" max="2019-05">
<script>
promise_test(() => {
let monthElement = document.getElementById('month');
let monthElement = document.getElementById('month0');
return openPickerWithPromise(monthElement)
.then(() => {
clickThisMonthButton();
......@@ -23,10 +25,42 @@ promise_test(() => {
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.");
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");
}, "Date picker: 'This month' button should select current month when clicked");
promise_test(() => {
let monthElement = document.getElementById('month1');
return openPickerWithPromise(monthElement)
.then(() => {
eventSender.keyDown('Tab'); // Tab over to the 'This month' button
eventSender.keyDown('Enter');
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 when activated with Enter key");
promise_test(() => {
let monthElement = document.getElementById('month2');
return openPickerWithPromise(monthElement)
.then(() => {
clickThisMonthButton();
assert_equals(monthElement.value, "2019-02", "Click on disabled 'This month' button shouldn't change date.");
assert_not_equals(internals.pagePopupWindow, null, "Click on disabled 'This month' button shouldn't close popup.");
eventSender.keyDown('Enter');
assert_not_equals(internals.pagePopupWindow, null, "Enter key should close popup.");
});
}, "Date picker: 'This month' button should be disabled when current month is not valid");
</script>
</body>
......
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