Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 89 additions & 7 deletions src/components/NcDateTimePicker/NcDateTimePicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Comment on lines +425 to +430
Copy link
Copy Markdown
Contributor

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.

<!-- default: true -->
<NcDateTimePicker />
<NcDateTimePicker :manualInput="false" />

<!-- default: false -->
<NcDateTimePicker />
<NcDateTimePicker noInput />
Suggested change
/**
* Enable or disable manual text input for the date picker.
*
* @default true
*/
manualInput?: boolean
/**
* Disable date text input
*/
noInput?: boolean

or

Suggested change
/**
* Enable or disable manual text input for the date picker.
*
* @default true
*/
manualInput?: boolean
/**
* Disable date text input
*/
noTextInput?: boolean

Copy link
Copy Markdown
Author

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:

  • False
  • True
  • Options {
    enterSubmit = true,
    tabSubmit = true,
    }

This way the options can be implemented later without having multiple props and/or breaking functionality.

Copy link
Copy Markdown
Contributor

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.

}>(), {
ariaLabel: t('Datepicker input'),
ariaLabelMenu: t('Datepicker menu'),
Expand All @@ -433,6 +440,7 @@ const props = withDefaults(defineProps<{
modelValue: null,
// set by fallbackPlaceholder
placeholder: undefined,
manualInput: true,
type: 'date',
inline: false,
})
Expand All @@ -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.
Expand Down Expand Up @@ -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
})

Expand Down Expand Up @@ -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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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.
*
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But not if the input value is changed significantly.

I enter 10/22/2026. Then I want to edit it changing the month, for example, 10 to 11. I go back and get Oct 22, 2026, where I actually need to change Oct to Nov.

@textSubmit="onTextSubmit">
<template #action-buttons>
<NcButton size="small" variant="tertiary" @click="cancelSelection">
{{ t('Cancel') }}
Expand Down
Loading