vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: Edit relative reminders #3248

Merged
konrad merged 27 commits from ce72/frontend:main into main 2023-06-10 17:04:10 +00:00
Conversation 18 Commits 27 Files Changed 18 +156 -221
7 changed files with 156 additions and 221 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 95487d7569 - Show all commits

1
src/components/input/datepickerInline.vue
View File

@ -151,7 +151,6 @@ function setDateValue(dateString: string | Date | null) {
function updateData() {
changed.value = true
console.log('emit', date.value)
emit('update:modelValue', date.value)
}

56
src/components/tasks/partials/reminder-detail.vue
View File

@ -3,29 +3,30 @@
dpschen marked this conversation as resolved Outdated
konrad commented 2023-03-28 09:57:47 +00:00
Outdated
Review
Copy Link

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.
dpschen commented 2023-03-30 10:58:37 +00:00
Outdated
Review
Copy Link

Fixed in my changes, see #3248 (comment)

Fixed in my changes, see https://kolaente.dev/vikunja/frontend/pulls/3248#issuecomment-47907
{{ reminderText }}
<div class="presets">
<div class="options" v-if="showFormSwitch === null">
<BaseButton
v-for="p in presets"
>
{{ formatReminder(p) }}
</BaseButton>
<BaseButton>
<BaseButton @click="showFormSwitch = 'relative'">
Custom
</BaseButton>
<BaseButton @click="showFormSwitch = 'absolute'">
Date
</BaseButton>
</div>
<ReminderPeriod
v-if="showRelativeReminder"
v-if="showFormSwitch === 'relative'"
v-model="reminder"
:disabled="disabled"
@update:modelValue="emit('update:modelValue', reminder.value)"
@update:modelValue="emit('update:modelValue', reminder)"
/>
<Datepicker
v-if="showAbsoluteReminder"
<DatepickerInline
v-if="showFormSwitch === 'absolute'"
v-model="reminderDate"
:disabled="disabled"
@close-on-change="setReminderDate"
@update:modelValue="setReminderDate"
/>
</div>
</template>
@ -34,16 +35,17 @@
import {computed, ref, watch, type PropType} from 'vue'
import {toRef} from '@vueuse/core'
import {SECONDS_A_DAY} from '@/constants/date'
import {secondsToPeriod} from '@/helpers/time/period'
import {REMINDER_PERIOD_RELATIVE_TO_TYPES} from '@/types/IReminderPeriodRelativeTo'
import {secondsToPeriod} from '@/helpers/time/period'
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
import {formatDateShort} from '@/helpers/time/formatDate'
import Datepicker from '@/components/input/datepicker.vue'
import ReminderPeriod from '@/components/tasks/partials/reminder-period.vue'
import TaskReminderModel from '@/models/taskReminder'
import BaseButton from '@/components/base/BaseButton.vue'
import {REMINDER_PERIOD_RELATIVE_TO_TYPES} from '@/types/IReminderPeriodRelativeTo'
import DatepickerInline from '@/components/input/datepickerInline.vue'
import ReminderPeriod from '@/components/tasks/partials/reminder-period.vue'
import TaskReminderModel from '@/models/taskReminder'
const props = defineProps({
modelValue: {
@ -65,20 +67,9 @@ const presets: TaskReminderModel[] = [
{relativePeriod: SECONDS_A_DAY * 7, relativeTo: REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE},
{relativePeriod: SECONDS_A_DAY * 30, relativeTo: REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE},
]
const reminderDate = computed({
get() {
return reminder.value?.reminder
},
set(newReminderDate) {
if (!reminderDate.value) {
return
}
reminder.value.reminder = new Date(reminderDate.value)
},
})
const reminderDate = ref(null)
const showAbsoluteReminder = computed(() => !reminder.value || !reminder.value?.relativeTo)
const showRelativeReminder = computed(() => !reminder.value || reminder.value?.relativeTo)
const showFormSwitch = ref<null | 'relative' | 'absolute'>(null)
const reminderText = computed(() => {
@ -103,10 +94,9 @@ watch(
)
function setReminderDate() {
if (!reminderDate.value) {
return
}
reminder.value.reminder = new Date(reminderDate.value)
reminder.value.reminder = reminderDate.value === null
? null
: new Date(reminderDate.value)
emit('update:modelValue', reminder.value)
}
@ -123,12 +113,12 @@ function formatReminder(reminder: TaskReminderModel) {
periodHuman = period.days + ' day'
}
return periodHuman + ' ' + (reminder.relativePeriod > 0 ? 'before' : 'after') + ' ' + reminder.relativeTo
return periodHuman + ' ' + (reminder.relativePeriod <= 0 ? 'before' : 'after') + ' ' + reminder.relativeTo
}
</script>
<style lang="scss" scoped>
.presets {
.options {
display: flex;
flex-direction: column;
align-items: flex-start;

232
src/components/tasks/partials/reminder-period.vue
View File

@ -1,92 +1,57 @@
<template>
<div
v-if="!!reminder?.relativeTo"
class="reminder-period"
class="reminder-period control"
>
ce72 marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:13:24 +00:00
Outdated
Review
Copy Link

What about inlining formatBeforeAfter to something like {{ reminder.relativePeriod <= 0 ? '&le;' : '&gt;' }}

What about inlining `formatBeforeAfter` to something like `{{ reminder.relativePeriod <= 0 ? '&le;' : '&gt;' }}`
ce72 commented 2023-03-17 09:36:03 +00:00
Outdated
Review
Copy Link

ok

ok
<Popup>
<template #trigger="{toggle}">
<BaseButton
@click="toggle"
:disabled="disabled"
class="show"
>
{{ formatDuration(reminder.relativePeriod) }} {{ reminder.relativePeriod <= 0 ? '&le;' : '&gt;' }}
{{ formatRelativeTo(reminder.relativeTo) }}
<span class="icon"><icon icon="chevron-down"/></span>
</BaseButton>
</template>
<input
class="input"
v-model.number="period.duration"
type="number"
min="0"
@change="updateData"
/>
<template #content>
<div class="mt-2">
<div class="control is-flex is-align-items-center">
<label>
<input
:disabled="disabled"
class="input"
:placeholder="$t('task.reminder.daysShort')"
v-model="periodInput.duration.days"
type="number"
min="0"
/> {{ $t('task.reminder.days') }}
</label>
<input
:disabled="disabled"
class="input"
:placeholder="$t('task.reminder.hoursShort')"
v-model="periodInput.duration.hours"
type="number"
min="0"
/>:
<input
:disabled="disabled"
class="input"
:placeholder="$t('task.reminder.minutesShort')"
v-model="periodInput.duration.minutes"
type="number"
min="0"
/>
<div class="select">
<select v-model="period.durationUnit" @change="updateData">
<option value="minutes">{{ $t('task.reminder.minutes') }}</option>
<option value="hours">{{ $t('task.reminder.hours') }}</option>
<option value="days">{{ $t('task.reminder.days') }}</option>
ce72 marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:09:27 +00:00
Outdated
Review
Copy Link

Please add a translation string for the d. (It does not look like much, but still)

Please add a translation string for the `d`. (It does not look like much, but still)
konrad commented 2023-03-16 19:09:48 +00:00
Outdated
Review
Copy Link

Same for the placeholders.

Same for the placeholders.
ce72 commented 2023-03-17 09:36:12 +00:00
Outdated
Review
Copy Link

ok

ok
dpschen commented 2023-03-20 16:35:18 +00:00
Outdated
Review
Copy Link

To be honest I was confused by the d when I first saw the interface.
Why don't we write it out?

To be honest I was confused by the `d` when I first saw the interface. Why don't we write it out?
ce72 commented 2023-03-20 20:29:34 +00:00
Outdated
Review
Copy Link

ok

ok
<option value="weeks">{{ $t('task.reminder.weeks') }}</option>
</select>
</div>
<div class="select">
<select :disabled="disabled" v-model.number="periodInput.sign">
<option value="-1">&le;</option>
<option value="1">&gt;</option>
</select>
</div>
<div class="select">
<select v-model.number="period.sign" @change="updateData">
<option value="-1">
before
</option>
<option value="1">
after
</option>
</select>
</div>
<div class="select">
<select :disabled="disabled" v-model="periodInput.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>
<div class="control">
<x-button
:disabled="disabled"
class="close-button"
:shadow="false"
@click="submitForm"
>
{{ $t('misc.confirm') }}
</x-button>
</div>
</div>
</template>
</Popup>
<div class="select">
<select v-model="period.relativeTo" @change="updateData">
<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>
</template>
<script setup lang="ts">
import {reactive, ref, watch, type PropType, computed} from 'vue'
import {ref, watch, type PropType} from 'vue'
import {useI18n} from 'vue-i18n'
import {toRef} from '@vueuse/core'
import BaseButton from '@/components/base/BaseButton.vue'
import Popup from '@/components/misc/popup.vue'
import {periodToSeconds, secondsToPeriod} from '@/helpers/time/period'
import {periodToSeconds, PeriodUnit, secondsToPeriod} from '@/helpers/time/period'
import TaskReminderModel from '@/models/taskReminder'
@ -108,106 +73,55 @@ const props = defineProps({
const emit = defineEmits(['update:modelValue'])
const reminder = ref<ITaskReminder>()
const reminder = ref<ITaskReminder>(new TaskReminderModel())
const showForm = ref(false)
const periodInput = reactive({
duration: {
days: 0,
hours: 0,
minutes: 0,
seconds: 0
},
interface PeriodInput {
duration: number,
durationUnit: PeriodUnit,
relativeTo: IReminderPeriodRelativeTo,
sign: -1 | 1,
}
const period = ref<PeriodInput>({
duration: 0,
durationUnit: 'hours',
relativeTo: REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE,
sign: -1,
})
const modelValue = toRef(props, 'modelValue')
watch(
modelValue,
(value) => {
reminder.value = value
if (value && value.relativeTo != null) {
Object.assign(periodInput.duration, secondsToPeriod(Math.abs(value.relativePeriod)))
periodInput.relativeTo = value.relativeTo
periodInput.sign = value.relativePeriod <= 0 ? -1 : 1
} else {
reminder.value = new TaskReminderModel()
showForm.value = true
}
},
{immediate: true},
modelValue,
(value) => {
console.log({value})
konrad commented 2023-03-28 10:01:23 +00:00
Outdated
Review
Copy Link

What does sign mean in this context?

What's the advantage of using an object here instead of individual refs?

What does `sign` mean in this context? What's the advantage of using an object here instead of individual `ref`s?
const p = secondsToPeriod(value?.relativePeriod)
period.value.durationUnit = p.unit
period.value.duration = p.amount
period.value.relativeTo = value?.relativeTo
},
{immediate: true},
)
function updateData() {
changed.value = true
if (reminder.value) {
reminder.value.relativePeriod = periodInput.sign * periodToSeconds(periodInput.duration.days, periodInput.duration.hours, periodInput.duration.minutes, 0)
reminder.value.relativeTo = periodInput.relativeTo
reminder.value.reminder = null
}
reminder.value.relativePeriod = period.value.sign * periodToSeconds(period.value.duration, period.value.durationUnit)
reminder.value.relativeTo = period.value.relativeTo
reminder.value.reminder = null
emit('update:modelValue', reminder.value)
}
function submitForm() {
updateData()
close()
}
const changed = ref(false)
function close() {
setTimeout(() => {
showForm.value = false
if (changed.value) {
changed.value = false
}
}, 200)
}
function formatDuration(reminderPeriod: number): string {
if (Math.abs(reminderPeriod) < 60) {
return '00:00'
}
const duration = secondsToPeriod(Math.abs(reminderPeriod))
return (duration.days > 0 ? `${duration.days} ${t('task.reminder.days')} `: '') +
('' + duration.hours).padStart(2, '0') + ':' +
('' + duration.minutes).padStart(2, '0')
}
const relativeToOptions = {
[REMINDER_PERIOD_RELATIVE_TO_TYPES.DUEDATE]: t('task.attributes.dueDate'),
[REMINDER_PERIOD_RELATIVE_TO_TYPES.STARTDATE]: t('task.attributes.startDate'),
[REMINDER_PERIOD_RELATIVE_TO_TYPES.ENDDATE]: t('task.attributes.endDate'),
} as const
const relativeTo = computed(() => relativeToOptions[periodInput.relativeTo])
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>
.input {
max-width: 5rem;
width: 4rem;
}
.reminder-period {
display: flex;
flex-direction: column;
gap: .25rem;
.close-button {
margin: 0.5rem;
width: calc(100% - 1rem);
.input, .select select {
width: 100% !important;
height: auto;
}
}
</style>

19
src/components/tasks/partials/reminders.vue
View File

@ -7,11 +7,14 @@
class="reminder-input"
>
<div class="reminder-detail">
<ReminderDetail :disabled="disabled" v-model="reminders[index]" />
<ReminderDetail
:disabled="disabled"
v-model="reminders[index]"
@update:model-value="updateData"/>
</div>
<div>
<BaseButton v-if="!disabled" @click="removeReminderByIndex(index)" class="remove">
<icon icon="times" />
<BaseButton v-if="!disabled" @click="removeReminderByIndex(index)" class="remove">
<icon icon="times"/>
</BaseButton>
</div>
</div>
@ -31,7 +34,7 @@
<script setup lang="ts">
import {reactive, ref, watch, type PropType} from 'vue'
konrad marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:15:53 +00:00
Outdated
Review
Copy Link

Why remove the type?

Why remove the type?
ce72 commented 2023-03-17 09:39:35 +00:00
Outdated
Review
Copy Link

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?
dpschen commented 2023-03-20 16:38:39 +00:00
Outdated
Review
Copy Link

@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?
ce72 commented 2023-03-20 20:32:47 +00:00
Outdated
Review
Copy Link

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.
dpschen commented 2023-03-27 21:47:44 +00:00
Outdated
Review
Copy Link

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.
ce72 commented 2023-03-28 05:54:51 +00:00
Outdated
Review
Copy Link

Then my question goes to @konrad

Then my question goes to @konrad
konrad commented 2023-03-28 10:04:44 +00:00
Outdated
Review
Copy Link

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 type { ITaskReminder } from '@/modelTypes/ITaskReminder'
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
import BaseButton from '@/components/base/BaseButton.vue'
import ReminderDetail from '@/components/tasks/partials/reminder-detail.vue'
@ -60,6 +63,7 @@ watch(
)
const isAddReminder = ref(false)
function toggleAddReminder() {
isAddReminder.value = !isAddReminder.value
}
@ -76,8 +80,9 @@ function editReminder(index: number) {
updateData()
}
function addNewReminder(newReminder : ITaskReminder) {
if (newReminder == null) {
function addNewReminder(newReminder: ITaskReminder) {
console.log('add new reminder', newReminder)
if (newReminder === null) {
return
}
reminders.value.push(newReminder)
@ -104,9 +109,11 @@ function removeReminderByIndex(index: number) {
margin-bottom: 0.75rem;
}
}
.reminder-detail {
width: 100%;
}
.remove {
color: var(--danger);
vertical-align: top;

46
src/helpers/time/period.ts
View File

@ -1,20 +1,50 @@
import {SECONDS_A_DAY, SECONDS_A_HOUR, SECONDS_A_MINUTE} from '@/constants/date'
import {
SECONDS_A_DAY,
SECONDS_A_HOUR,
SECONDS_A_MINUTE,
SECONDS_A_MONTH,
SECONDS_A_WEEK,
dpschen marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:16:50 +00:00
Outdated
Review
Copy Link

Can you add a type hint for the return value?

Can you add a type hint for the return value?
ce72 commented 2023-03-17 09:39:43 +00:00
Outdated
Review
Copy Link

ok

ok
dpschen commented 2023-03-20 16:48:28 +00:00
Outdated
Review
Copy Link

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
ce72 commented 2023-03-20 20:33:22 +00:00
Outdated
Review
Copy Link

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.
👍 1
SECONDS_A_YEAR,
} from '@/constants/date'
export type PeriodUnit = 'seconds' | 'minutes' | 'hours' | 'days' | 'weeks' | 'months' | 'years'
/**
* Convert time period given as seconds to days, hour, minutes, seconds
*/
export function secondsToPeriod(seconds: number): {days: number, hours: number, minutes: number, seconds: number} {
export function secondsToPeriod(seconds: number): { unit: PeriodUnit, amount: number } {
if (seconds % SECONDS_A_DAY === 0) {
if (seconds % SECONDS_A_WEEK === 0) {
return {unit: 'weeks', amount: seconds / SECONDS_A_WEEK}
} else if (seconds % SECONDS_A_MONTH === 0) {
return {unit: 'months', amount: seconds / SECONDS_A_MONTH}
} else if (seconds % SECONDS_A_YEAR === 0) {
return {unit: 'years', amount: seconds / SECONDS_A_YEAR}
} else {
return {unit: 'days', amount: seconds / SECONDS_A_DAY}
}
}
return {
days: Math.floor(seconds / SECONDS_A_DAY),
hours: Math.floor(seconds % SECONDS_A_DAY / 3600),
minutes: Math.floor(seconds % SECONDS_A_HOUR / 60),
seconds: Math.floor(seconds % 60),
unit: 'hours',
amount: seconds / SECONDS_A_HOUR,
}
}
/**
* 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
export function periodToSeconds(period: number, unit: PeriodUnit): number {
switch (unit) {
case 'minutes':
return period * SECONDS_A_MINUTE
case 'hours':
return period * SECONDS_A_HOUR
case 'days':
return period * SECONDS_A_DAY
case 'weeks':
return period * SECONDS_A_WEEK
}
return 0
}

21
src/models/task.ts
View File

@ -22,6 +22,7 @@ import AttachmentModel from './attachment'
import SubscriptionModel from './subscription'
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
import TaskReminderModel from '@/models/taskReminder'
import {periodToSeconds, secondsToPeriod} from '@/helpers/time/period'
export const TASK_DEFAULT_COLOR = '#1973ff'
@ -37,21 +38,13 @@ export function getHexColor(hexColor: string): string {
* Parses `repeatAfterSeconds` into a usable js object.
*/
export function parseRepeatAfter(repeatAfterSeconds: number): IRepeatAfter {
let repeatAfter: IRepeatAfter = {type: 'hours', amount: repeatAfterSeconds / SECONDS_A_HOUR}
// if its dividable by 24, its something with days, otherwise hours
if (repeatAfterSeconds % SECONDS_A_DAY === 0) {
if (repeatAfterSeconds % SECONDS_A_WEEK === 0) {
repeatAfter = {type: 'weeks', amount: repeatAfterSeconds / SECONDS_A_WEEK}
} else if (repeatAfterSeconds % SECONDS_A_MONTH === 0) {
repeatAfter = {type:'months', amount: repeatAfterSeconds / SECONDS_A_MONTH}
} else if (repeatAfterSeconds % SECONDS_A_YEAR === 0) {
repeatAfter = {type: 'years', amount: repeatAfterSeconds / SECONDS_A_YEAR}
} else {
repeatAfter = {type: 'days', amount: repeatAfterSeconds / SECONDS_A_DAY}
}
const period = secondsToPeriod(repeatAfterSeconds)
return {
type: period.unit,
amount: period.amount,
}
return repeatAfter
}
export default class TaskModel extends AbstractModel<ITask> implements ITask {

2
src/types/IRepeatAfter.ts
View File

@ -1,4 +1,6 @@
export const REPEAT_TYPES = {
Seconds: 'seconds',
Minutes: 'minutes',
Hours: 'hours',
Days: 'days',
Weeks: 'weeks',