feat: Edit relative reminders #3248

Merged
konrad merged 27 commits from ce72/frontend:main into main 2023-06-10 17:04:10 +00:00
4 changed files with 243 additions and 9 deletions
Showing only changes of commit 5d38b8327f - Show all commits

View File

@ -0,0 +1,211 @@
<template>
<div class="reminder-detail">
<BaseButton class="show" v-if="!!reminder?.relativePeriod" @click.stop="togglePeriodPopup"
dpschen marked this conversation as resolved Outdated

Nitpicking, but please format this like this:

<ReminderPeriod 
    v-if="showRelativeReminder"
    v-model="reminder"
    :disabled="disabled"
    @update:modelValue="() => updateData()"
/>

That way it's more readable.

Nitpicking, but please format this like this: ``` <ReminderPeriod v-if="showRelativeReminder" v-model="reminder" :disabled="disabled" @update:modelValue="() => updateData()" /> ``` That way it's more readable.

Fixed in my changes, see #3248 (comment)

Fixed in my changes, see https://kolaente.dev/vikunja/frontend/pulls/3248#issuecomment-47907
:disabled="disabled || undefined">
{{ formatDuration(reminder.relativePeriod) }} <span v-html="formatBeforeAfter(reminder.relativePeriod)"></span>
{{ formatRelativeTo(reminder.relativeTo) }}
</BaseButton>
<CustomTransition name="fade">
<div v-if="show" class="control is-flex is-align-items-center mb-2">
<input
:disabled="disabled || undefined"
class="input"
placeholder="d"
v-model="periodInput.duration.days"
type="number"
min="0"
/> d
<input
:disabled="disabled || undefined"
class="input"
placeholder="HH"
v-model="periodInput.duration.hours"
type="number"
min="0"
/>:
<input
:disabled="disabled || undefined"
class="input"
placeholder="MM"
v-model="periodInput.duration.minutes"
type="number"
min="0"
/>
<div class="select">
<select v-model="periodInput.sign" id="sign">
<option value="-1">&le;</option>
<option value="1">&gt;</option>
</select>
</div>
<div class="control">
<div class="select">
<select v-model="periodInput.relativeTo" id="relativeTo">
<option :value="REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE">{{ $t('task.attributes.dueDate') }}</option>
<option :value="REMINDER_PERIOD_RELATIVE_TO_TYPES.STARTDATE">{{ $t('task.attributes.startDate') }}</option>
<option :value="REMINDER_PERIOD_RELATIVE_TO_TYPES.ENDDATE">{{ $t('task.attributes.endDate') }}</option>
</select>
</div>
</div>
<x-button
class="datepicker__close-button"
:shadow="false"
@click="close"
v-cy="'closeDatepicker'"
>
dpschen marked this conversation as resolved Outdated

Please use a computed for both show... functions instead.

Please use a `computed` for both `show...` functions instead.
Outdated
Review

ok

ok
{{ $t('misc.confirm') }}
</x-button>
</div>
</CustomTransition>
</div>
</template>
<script setup lang="ts">
import {reactive, ref, watch, type PropType} from 'vue'
import BaseButton from '@/components/base/BaseButton.vue'
import CustomTransition from '@/components/misc/CustomTransition.vue'
import {periodToSeconds, secondsToPeriod} from '@/helpers/time/period'
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
import {REMINDER_PERIOD_RELATIVE_TO_TYPES, type IReminderPeriodRelativeTo} from '@/types/IReminderPeriodRelativeTo'
import {useI18n} from 'vue-i18n'
const {t} = useI18n({useScope: 'global'})
const props = defineProps({
modelValue: {
type: Object as PropType<ITaskReminder>,
required: true,
},
disabled: {
type: Boolean,
default: false,
},
})
const emit = defineEmits(['update:modelValue', 'close', 'close-on-change'])
const reminder = ref<ITaskReminder>()
const show = ref(false)
const periodInput = reactive({
duration: {days: 0, hours: 0, minutes: 0, seconds: 0},
relativeTo: REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE,
sign: -1,
})
watch(
() => props.modelValue,
(value) => {
reminder.value = value
if (value.relativePeriod != 0) {
Object.assign(periodInput.duration, secondsToPeriod(Math.abs(value.relativePeriod)))
periodInput.relativeTo = value.relativeTo
periodInput.sign = value.relativePeriod <= 0 ? -1 : 1
}
},
{immediate: true},
)
function updateData() {
changed.value = true
console.log('updateData', periodInput)
reminder.value.relativePeriod = parseInt(periodInput.sign) * periodToSeconds(periodInput.duration.days, periodInput.duration.hours, periodInput.duration.minutes, 0)
reminder.value.relativeTo = periodInput.relativeTo
reminder.value.reminder = null
emit('update:modelValue', reminder.value)
}
function togglePeriodPopup() {
if (props.disabled) {
return
}
show.value = !show.value
}
const changed = ref(false)
function close() {
// Kind of dirty, but the timeout allows us to enter a time and click on "confirm" without
// having to click on another input field before it is actually used.
updateData()
setTimeout(() => {
show.value = false
emit('close', changed.value)
if (changed.value) {
changed.value = false
emit('close-on-change', changed.value)
}
}, 200)
}
function formatDuration(reminderPeriod: number): string {
if (Math.abs(reminderPeriod) < 60) {
return '00:00'
}
const duration = secondsToPeriod(Math.abs(reminderPeriod))
console.log('formatDuration', duration, reminderPeriod)
return (duration.days > 0 ? duration.days + ' d ' : '') +
('' + duration.hours).padStart(2, '0') + ':' +
('' + duration.minutes).padStart(2, '0')
}
function formatBeforeAfter(reminderPeriod: number): string {
if (reminderPeriod <= 0) {
return '&le;'
}
return '&gt;'
}
function formatRelativeTo(relativeTo: IReminderPeriodRelativeTo | null): string | null {
switch (relativeTo) {
case REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE:
return t('task.attributes.dueDate')
case REMINDER_PERIOD_RELATIVE_TO_TYPES.STARTDATE:
return t('task.attributes.startDate')
case REMINDER_PERIOD_RELATIVE_TO_TYPES.ENDDATE:
return t('task.attributes.endDate')
default:
return relativeTo
}
}
</script>
<style lang="scss" scoped>
.reminders {
.reminder-input {
display: flex;
align-items: center;
&.overdue :deep(.datepicker .show) {
color: var(--danger);
}
&:last-child {
margin-bottom: 0.75rem;
}
.remove {
color: var(--danger);
padding-left: .5rem;
}
}
}
.input {
max-width: 70px;
width: 70px;
}
.datepicker__close-button {
margin: 1rem;
width: calc(100% - 2rem);
}
</style>

View File

@ -6,7 +6,10 @@
:class="{ 'overdue': r.reminder < new Date()}"
class="reminder-input"
>
<ReminderDetail v-if="!!r.relativePeriod" v-model="reminders[index]" @close-on-change="() => addReminder(index)"
/>
<Datepicker
v-if="!r.relativePeriod"
v-model="reminders[index].reminder"
:disabled="disabled"
@close-on-change="() => addReminderDate(index)"
@ -26,10 +29,11 @@
</template>
<script setup lang="ts">
import {type PropType, ref, onMounted, watch} from 'vue'
import { onMounted, ref, watch, type PropType } from 'vue'
import BaseButton from '@/components/base/BaseButton.vue'
import Datepicker from '@/components/input/datepicker.vue'
import ReminderDetail from '@/components/tasks/partials/reminder-detail.vue'
konrad marked this conversation as resolved Outdated

Why remove the type?

Why remove the type?
Outdated
Review

The type has changed from Date | String to a complex type (Array as PropType<ITaskReminder[]>). I don't see why a validation is required. What do you suggest?

The type has changed from `Date | String` to a complex type (`Array as PropType<ITaskReminder[]>`). I don't see why a validation is required. What do you suggest?

@ce72: Sry I don't understand your answer here :)
What has the validation of the modelValue to do with the type of disabled?

@ce72: Sry I don't understand your answer here :) What has the validation of the modelValue to do with the type of disabled?
Outdated
Review

Can you please make a concrete suggestion what you expect here? I can find no other place in the code where you have a validation on a complex props type. And am not enough typescript native to understand what you want to have added here.

Can you please make a concrete suggestion what you expect here? I can find no other place in the code where you have a validation on a complex props type. And am not enough typescript native to understand what you want to have added here.

I think I misunderstood @konrads first question, because the lines changed. I assumed he is refering to the removed type from disabled and not the removed validation of the modelValue.

I think I misunderstood @konrads first question, because the lines changed. I assumed he is refering to the removed type from disabled and not the removed validation of the modelValue.
Outdated
Review

Then my question goes to @konrad

Then my question goes to @konrad

The type has changed from Date | String to a complex type (Array as PropType<ITaskReminder[]>). I don't see why a validation is required. What do you suggest?

Okay that actually makes sense.

> The type has changed from Date | String to a complex type (Array as PropType<ITaskReminder[]>). I don't see why a validation is required. What do you suggest? Okay that actually makes sense.
import TaskReminderModel from '@/models/taskReminder'
import type { ITaskReminder } from '@/modelTypes/ITaskReminder'
@ -39,11 +43,7 @@ const props = defineProps({
default: () => [],
validator(prop) {
// This allows arrays
if (!(prop instanceof Array)) {
return false
}
return true
return prop instanceof Array
},
},
disabled: {
@ -78,7 +78,11 @@ function updateData() {
}
const newReminder = ref(null)
function addReminderDate(index : number | null = null) {
function addReminder(index: number | null = null) {
updateData()
}
function addReminderDate(index: number | null = null) {
// New Date
if (index === null) {
if (newReminder.value === null) {

View File

@ -0,0 +1,19 @@
import {SECONDS_A_DAY, SECONDS_A_HOUR, SECONDS_A_MINUTE} from '@/constants/date'
/**
* Convert time period given as seconds to days, hour, minutes, seconds
*/
export function secondsToPeriod(seconds: number) {
dpschen marked this conversation as resolved Outdated

Can you add a type hint for the return value?

Can you add a type hint for the return value?
Outdated
Review

ok

ok

Couldn't we instead define the return via as const?
Also we don' really need the declaration of the variables but could inline those definitions in the returned object directly

Couldn't we instead define the return via `as const`? Also we don' really need the declaration of the variables but could inline those definitions in the returned object directly
Outdated
Review

OK concerning the inlining. Can you please make a suggestion on how to replace the function using as const? My typescript might be not sufficient here.

OK concerning the inlining. Can you please make a suggestion on how to replace the function using `as const`? My typescript might be not sufficient here.
const d = Math.floor(seconds / SECONDS_A_DAY)
const h = Math.floor(seconds % SECONDS_A_DAY / 3600)
const m = Math.floor(seconds % SECONDS_A_HOUR / 60)
const s = Math.floor(seconds % 60)
return {days: d, hours: h, minutes: m, seconds: s}
}
/**
* Convert time period of days, hour, minutes, seconds to duration in seconds
*/
export function periodToSeconds(days: number, hours: number, minutes: number, seconds: number) : number{
return days * SECONDS_A_DAY + hours * SECONDS_A_HOUR + minutes * SECONDS_A_MINUTE + seconds
}

View File

@ -2,7 +2,7 @@ import type { IAbstract } from './IAbstract'
import type { IReminderPeriodRelativeTo } from '@/types/IReminderPeriodRelativeTo'
export interface ITaskReminder extends IAbstract {
reminder: Date
reminder: Date | null
relativePeriod: number
relativeTo: IReminderPeriodRelativeTo
relativeTo: IReminderPeriodRelativeTo | null
}