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 +316 -218
3 changed files with 316 additions and 218 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0d6c0c8399 - Show all commits

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

@ -1,179 +1,79 @@
<template>
<div class="reminder-detail">
<BaseButton class="show" v-if="!!reminder?.relativePeriod" @click.stop="togglePeriodPopup"
: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'"
>
{{ $t('misc.confirm') }}
</x-button>
</div>
</CustomTransition>
<ReminderPeriod v-if="showRelativeReminder()" v-model="reminder" @update:modelValue="() => updateData()"></ReminderPeriod>
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
<Datepicker
v-if="showAbsoluteReminder()"
v-model="reminderDate"
:disabled="disabled"
@close-on-change="() => setReminderDate()"
/>
</div>
</template>
<script setup lang="ts">
import {reactive, ref, watch, type PropType} from 'vue'
import { 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'})
import Datepicker from '@/components/input/datepicker.vue'
import ReminderPeriod from '@/components/tasks/partials/reminder-period.vue'
import TaskReminderModel from '@/models/taskReminder'
import type { ITaskReminder } from '@/modelTypes/ITaskReminder'
const props = defineProps({
modelValue: {
type: Object as PropType<ITaskReminder>,
required: true,
required: false,
},
disabled: {
type: Boolean,
default: false,
},
})
const emit = defineEmits(['update:modelValue', 'close', 'close-on-change'])
const emit = defineEmits(['update:modelValue', 'update:Reminder', '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,
})
const reminderDate = ref()
watch(
() => props.modelValue,
(value) => {
() => props.modelValue,
(value) => {
console.log('reminder-detail.watch', 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
if (reminder.value && reminder.value.reminder) {
reminderDate.value = new Date(reminder.value.reminder)
}
},
{immediate: true},
)
function setReminderDate() {
console.log('reminder-detail.setReminderDate', reminderDate.value)
console.log('reminder-detail.setReminderDate.reminder', reminder.value)
if (!reminderDate.value) {
return
}
if (!reminder.value) {
dpschen marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:08:53 +00:00
Outdated
Review
Copy Link

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

Please use a `computed` for both `show...` functions instead.
ce72 commented 2023-03-17 09:35:55 +00:00
Outdated
Review
Copy Link

ok

ok
reminder.value = new TaskReminderModel()
}
reminder.value.reminder = new Date(reminderDate.value)
updateData()
}
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
console.log('reminder-detail.updateData', reminder.value)
emit('update:modelValue', reminder.value)
}
function togglePeriodPopup() {
if (props.disabled) {
return
}
show.value = !show.value
function showAbsoluteReminder() {
return !reminder.value || !reminder.value?.relativeTo
}
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 showRelativeReminder() {
console.log('showRelativeReminder', reminder.value)
return !reminder.value || reminder.value?.relativeTo
}
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>
@ -197,15 +97,4 @@ function formatRelativeTo(relativeTo: IReminderPeriodRelativeTo | null): string
}
}
.input {
max-width: 70px;
width: 70px;
}
.datepicker__close-button {
margin: 1rem;
width: calc(100% - 2rem);
}
</style>

218
src/components/tasks/partials/reminder-period.vue Normal file
View File

@ -0,0 +1,218 @@
<template>
<div class="datepicker">
<BaseButton class="show" v-if="!!reminder?.relativeTo" @click.stop="togglePeriodPopup"
:disabled="disabled || undefined">
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
{{ 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
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
<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'"
>
{{ $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 TaskReminderModel from '@/models/taskReminder'
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: false,
},
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,
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?
sign: -1,
})
watch(
() => props.modelValue,
(value) => {
console.log('reminders-period.watch', 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()
show.value = true
}
},
{immediate: true},
)
function updateData() {
changed.value = true
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
console.log('reminders-period.updateData', reminder.value)
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))
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;
}
dpschen marked this conversation as resolved Outdated
konrad commented 2023-03-16 19:14:30 +00:00
Outdated
Review
Copy Link

Please use `rem

Please use `rem
ce72 commented 2023-03-17 09:36:21 +00:00
Outdated
Review
Copy Link

ok

ok
.remove {
color: var(--danger);
padding-left: .5rem;
}
}
}
.input {
max-width: 70px;
width: 70px;
}
.datepicker__close-button {
margin: 1rem;
width: calc(100% - 2rem);
}
</style>

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

@ -1,53 +1,43 @@
<template>
<div class="reminders">
<div
v-for="(r, index) in reminders"
:key="index"
: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)"
/>
<BaseButton @click="removeReminderByIndex(index)" v-if="!disabled" class="remove">
<icon icon="times"></icon>
</BaseButton>
<div v-for="(r, index) in reminders" :key="index" :class="{ 'overdue': r.reminder < new Date() }" class="reminder-input">
<div>
<ReminderDetail :disabled="disabled" v-model="reminders[index]" mode="edit"
@update:modelValue="() => editReminder(index)"/>
</div>
<div>
<BaseButton @click="removeReminderByIndex(index)" v-if="!disabled" class="remove">
<icon icon="times"></icon>
</BaseButton>
</div>
</div>
<div class="reminder-input" v-if="!disabled">
<Datepicker
v-model="newReminder"
@close-on-change="() => addReminderDate()"
:choose-date-label="$t('task.addReminder')"
/>
<BaseButton @click.stop="toggleAddReminder" class="show" :disabled="disabled || undefined">
{{ $t('task.addReminder') }}
</BaseButton>
<CustomTransition name="fade">
<ReminderDetail v-if="isAddReminder" :disabled="disabled" mode="add" @update:modelValue="addNewReminder"/>
</CustomTransition>
</div>
</div>
</template>
<script setup lang="ts">
import { onMounted, ref, watch, type PropType } from 'vue'
import {onMounted, reactive, ref, watch, type PropType} from 'vue'
import BaseButton from '@/components/base/BaseButton.vue'
import Datepicker from '@/components/input/datepicker.vue'
import CustomTransition from '@/components/misc/CustomTransition.vue'
import ReminderDetail from '@/components/tasks/partials/reminder-detail.vue'
import TaskReminderModel from '@/models/taskReminder'
import type { ITaskReminder } from '@/modelTypes/ITaskReminder'
import type {ITaskReminder} from '@/modelTypes/ITaskReminder'
const props = defineProps({
modelValue: {
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.
type: Array as PropType<ITaskReminder[]>,
default: () => [],
validator(prop) {
// This allows arrays
return prop instanceof Array
},
},
disabled: {
type: Boolean,
default: false,
},
})
@ -61,42 +51,42 @@ onMounted(() => {
})
watch(
() => props.modelValue,
(newVal) => {
for (const i in newVal) {
if (typeof newVal[i].reminder === 'string') {
newVal[i].reminder = new Date(newVal[i].reminder)
}
}
reminders.value = newVal
},
() => props.modelValue,
(newVal) => {
reminders.value = newVal
},
)
const isAddReminder = ref(false)
function toggleAddReminder() {
isAddReminder.value = !isAddReminder.value
}
function updateData() {
console.log('reminders.updateData', reminders.value)
emit('update:modelValue', reminders.value)
isAddReminder.value = false
}
const newReminder = ref(null)
function addReminder(index: number | null = null) {
updateData()
}
function addReminderDate(index: number | null = null) {
// New Date
if (index === null) {
if (newReminder.value === null) {
return
}
reminders.value.push(new TaskReminderModel({reminder: new Date(newReminder.value)}))
newReminder.value = null
} else if(reminders.value[index].reminder === null) {
function editReminder(index: number) {
console.log('reminders.editReminder', reminders.value[index])
if (reminders.value[index] === null) {
return
}
updateData()
}
function addNewReminder(newReminder) {
console.log('reminders.addNewReminder to old reminders', newReminder)
if (newReminder == null) {
return
}
reminders.value.push(newReminder)
newReminder = reactive(new TaskReminderModel())
updateData()
}
function removeReminderByIndex(index: number) {
reminders.value.splice(index, 1)
updateData()
@ -105,22 +95,23 @@ function removeReminderByIndex(index: number) {
<style lang="scss" scoped>
.reminders {
.reminder-input {
display: flex;
align-items: center;
.reminder-input {
display: flex;
align-items: center;
&.overdue :deep(.datepicker .show) {
color: var(--danger);
}
&.overdue :deep(.datepicker .show) {
color: var(--danger);
}
&:last-child {
margin-bottom: 0.75rem;
}
&:last-child {
margin-bottom: 0.75rem;
}
.remove {
color: var(--danger);
padding-left: .5rem;
}
}
.remove {
color: var(--danger);
padding-left: 2rem;
}
}
}
</style>