-
Notifications
You must be signed in to change notification settings - Fork 189
Fix: Improve date input UX and prevent manual editing issues #591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,15 +79,15 @@ function setupButtonTooltips() { | |
| } | ||
|
|
||
| function getToday() { | ||
| const today = new Date(); | ||
| return today.toISOString().split('T')[0]; | ||
| const now = new Date(); | ||
| return `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}`; | ||
| } | ||
|
|
||
| function getYesterday() { | ||
| const today = new Date(); | ||
| const yesterday = new Date(today); | ||
| yesterday.setDate(today.getDate() - 1); | ||
| return yesterday.toISOString().split('T')[0]; | ||
| return `${yesterday.getFullYear()}-${String(yesterday.getMonth() + 1).padStart(2, '0')}-${String(yesterday.getDate()).padStart(2, '0')}`; | ||
| } | ||
|
|
||
| function applyI18n() { | ||
|
|
@@ -1057,18 +1057,38 @@ document.addEventListener('DOMContentLoaded', () => { | |
| yesterdayRadio.addEventListener('change', () => { | ||
| browser.storage.local.set({ yesterdayContribution: yesterdayRadio.checked }); | ||
| }); | ||
| startingDateInput.addEventListener('input', () => { | ||
| window.scrumDateRangeUtils.normalizeSyncAndPersistDateRange( | ||
| startingDateInput, | ||
| endingDateInput, | ||
| ); | ||
|
|
||
| startingDateInput.addEventListener('change', () => { | ||
| window.scrumDateRangeUtils.normalizeSyncAndPersistDateRange( | ||
| startingDateInput, | ||
| endingDateInput | ||
|
Comment on lines
+1061
to
+1064
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Consider also handling keyboard-based interactions for opening the date picker. At the moment the picker only opens on Suggested implementation: startingDateInput.addEventListener('change', () => {
window.scrumDateRangeUtils.normalizeSyncAndPersistDateRange(
startingDateInput,
endingDateInput,
);
});
// Improve keyboard accessibility for the date picker on the starting date input
startingDateInput.addEventListener('keydown', (event) => {
if (event.key === 'Enter' || event.key === ' ') {
// Some browsers expose showPicker on input[type="date"]
if (typeof startingDateInput.showPicker === 'function') {
event.preventDefault();
startingDateInput.showPicker();
}
}
});
|
||
| ); | ||
| }); | ||
| endingDateInput.addEventListener('input', () => { | ||
| window.scrumDateRangeUtils.normalizeSyncAndPersistDateRange( | ||
| startingDateInput, | ||
| endingDateInput, | ||
| ); | ||
|
|
||
| endingDateInput.addEventListener('change', () => { | ||
| window.scrumDateRangeUtils.normalizeSyncAndPersistDateRange( | ||
| startingDateInput, | ||
| endingDateInput | ||
| ); | ||
| }); | ||
|
|
||
|
|
||
| startingDateInput.addEventListener('click', () => { | ||
| if (typeof startingDateInput.showPicker === 'function') { | ||
| startingDateInput.showPicker(); | ||
| } | ||
| }); | ||
|
|
||
| endingDateInput.addEventListener('click', () => { | ||
| if (typeof endingDateInput.showPicker === 'function') { | ||
| endingDateInput.showPicker(); | ||
| } | ||
| }); | ||
|
|
||
| const now = new Date(); | ||
| const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String(now.getDate()).padStart(2, '0')}`; | ||
| startingDateInput.max = today; | ||
| endingDateInput.max = today; | ||
|
|
||
| // Save username to storage on input and update button state | ||
| platformUsername.addEventListener('input', () => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Combining
readonlywithshowPickercan make the field unusable whereshowPickeris unsupported.Because the inputs are
readonlyand only open viashowPicker, environments withoutHTMLInputElement.prototype.showPickerwill leave them non-interactive, so users can’t set dates. Consider conditionally removingreadonlywhenshowPickeris unavailable or providing an alternate date input UI in that case.