-
Notifications
You must be signed in to change notification settings - Fork 97
feat(NcDateTimePicker): manual date input #8443
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 |
|---|---|---|
|
|
@@ -288,7 +288,7 @@ import { | |
| getFirstDay, | ||
| } from '@nextcloud/l10n' | ||
| import VueDatePicker from '@vuepic/vue-datepicker' | ||
| import { computed, useTemplateRef } from 'vue' | ||
| import { computed, ref, useTemplateRef } from 'vue' | ||
| import NcIconSvgWrapper from '../NcIconSvgWrapper/NcIconSvgWrapper.vue' | ||
| import NcTimezonePicker from '../NcTimezonePicker/NcTimezonePicker.vue' | ||
| import { t } from '../../l10n.ts' | ||
|
|
@@ -421,6 +421,13 @@ const props = withDefaults(defineProps<{ | |
| * @default false | ||
| */ | ||
| inline?: boolean | ||
|
|
||
| /** | ||
| * Enable or disable manual text input for the date picker. | ||
| * | ||
| * @default true | ||
| */ | ||
| manualInput?: boolean | ||
| }>(), { | ||
| ariaLabel: t('Datepicker input'), | ||
| ariaLabelMenu: t('Datepicker menu'), | ||
|
|
@@ -433,6 +440,7 @@ const props = withDefaults(defineProps<{ | |
| modelValue: null, | ||
| // set by fallbackPlaceholder | ||
| placeholder: undefined, | ||
| manualInput: true, | ||
| type: 'date', | ||
| inline: false, | ||
| }) | ||
|
|
@@ -453,6 +461,7 @@ const emit = defineEmits<{ | |
|
|
||
| const targetElement = useTemplateRef('target') | ||
| const pickerInstance = useTemplateRef('picker') | ||
| const manualInputValue = ref<Date | null>(null) | ||
|
|
||
| /** | ||
| * Mapping of the model-value prop to the format expected by the library. | ||
|
|
@@ -559,12 +568,17 @@ const realFormat = computed<LibraryFormatOptions>(() => { | |
| } | ||
|
|
||
| if (formatter) { | ||
| return (input: Date | [Date, Date]) => Array.isArray(input) | ||
| ? formatter.formatRange(input[0], input[1]) | ||
| : formatter.format(input) | ||
| return (input: Date | [Date, Date]) => { | ||
| if (Array.isArray(input)) { | ||
| return isFiniteDateRange(input) | ||
| ? formatter.formatRange(input[0], input[1]) | ||
| : '' | ||
| } | ||
|
|
||
| return isFiniteDate(input) ? formatter.format(input) : '' | ||
| } | ||
| } | ||
|
|
||
| // fallback to default formatting | ||
| return undefined | ||
| }) | ||
|
|
||
|
|
@@ -627,6 +641,50 @@ function onUpdateModelValue(value: LibraryModelValue): void { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Propagate valid manually typed input through the same model normalization path as picker selections. | ||
| * | ||
| * @param event The raw input event | ||
| * @param parsedDate The library-parsed date or null if parsing failed | ||
| */ | ||
| function onTextInput(event: Event, parsedDate: Date | null): void { | ||
| if (parsedDate) { | ||
| manualInputValue.value = parsedDate | ||
| return | ||
| } | ||
|
|
||
| const input = event.target instanceof HTMLInputElement ? event.target.value : '' | ||
| const tempDate = new Date(input) | ||
|
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. It works with the date input and date-time input, but it breaks on the week input, the month input, the year input and the range input.
Author
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. Oh, will look in to it. |
||
| manualInputValue.value = Number.isNaN(tempDate.getTime()) ? null : tempDate | ||
|
|
||
| if (manualInputValue.value !== null && pickerInstance.value) { | ||
| pickerInstance.value.updateInternalModelValue(manualInputValue.value) | ||
| } | ||
| } | ||
|
|
||
| function onTextSubmit(): void { | ||
| commitManualInput() | ||
| } | ||
|
|
||
| function onManualInputKeySubmit(): void { | ||
| commitManualInput() | ||
| } | ||
|
|
||
| function onBlur(): void { | ||
| commitManualInput() | ||
| emit('blur') | ||
| } | ||
|
|
||
| function commitManualInput(): void { | ||
| if (manualInputValue.value === null) { | ||
| return | ||
| } | ||
|
|
||
| onUpdateModelValue(manualInputValue.value) | ||
| manualInputValue.value = null | ||
| pickerInstance.value?.closeMenu() | ||
| } | ||
|
|
||
| /** | ||
| * Format a vuepick time object to native JS Date object. | ||
| * | ||
|
|
@@ -762,6 +820,26 @@ function sameDay(a: Date, b: Date): boolean { | |
| && a.getDate() === b.getDate() | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param value | ||
| */ | ||
| function isFiniteDate(value: unknown): value is Date { | ||
| return value instanceof Date && Number.isFinite(value.getTime()) | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param value | ||
| */ | ||
| function isFiniteDateRange(value: unknown): value is [Date, Date] { | ||
| return Array.isArray(value) | ||
| && value.length === 2 | ||
| && isFiniteDate(value[0]) | ||
| && isFiniteDate(value[1]) | ||
| } | ||
|
|
||
| </script> | ||
|
|
||
| <template> | ||
|
|
@@ -789,13 +867,17 @@ function sameDay(a: Date, b: Date): boolean { | |
| sixWeeks="fair" | ||
| :inline | ||
| :teleport="appendToBody ? (targetElement || undefined) : false" | ||
| textInput | ||
| :textInput="manualInput" | ||
| :weekNumName | ||
| :weekNumbers="showWeekNumber ? { type: 'iso' } : undefined" | ||
| :weekStart | ||
| v-bind="pickerType" | ||
| @update:modelValue="onUpdateModelValue" | ||
| @blur="emit('blur')"> | ||
| @blur="onBlur" | ||
| @keydown.enter.capture="onManualInputKeySubmit" | ||
| @keydown.tab.capture="onManualInputKeySubmit" | ||
| @textInput="onTextInput" | ||
|
Comment on lines
+877
to
+879
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. This breaks accessibility by preventing using the data picker (the menu popup) via keyboard. Previously, when the focus was in the input field, the menu could be open by typing or pressing Space, and then moving the focus to the menu by Tab Now, Space acts as text input, and Tab goes to the next element, without opening the menu.
Author
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. If a user can type in a date, this is much easier then trying to navigate a popup menu with a keyboard, so I don't seem this as a problem. Alternatively we could disable the tab capture and a user can tab in to the menu. I was actually thinking of disabling the popup menu on manual input, but thought is was a good visual feed back that the correct date is selected.
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. But not if the input value is changed significantly. I enter |
||
| @textSubmit="onTextSubmit"> | ||
| <template #action-buttons> | ||
| <NcButton size="small" variant="tertiary" @click="cancelSelection"> | ||
| {{ t('Cancel') }} | ||
|
|
||
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.
We follow no-boolean-default props as more natural for HTML-styled template, allowing using boolean shortcut like in HTML.
or
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.
I left this as manualInput with a boolean on purpose, the underlying component actually supports:
enterSubmit = true,
tabSubmit = true,
}
This way the options can be implemented later without having multiple props and/or breaking functionality.
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.
I agree with Grigorii here, we should stick with no boolean default rather than some underlying library.
In general we should not care about the library we wrap as we should define an interface and translate this to the library and not the otherway around.