WIP: feat: add default reminder for tasks with a due date #2360

Draft
konrad wants to merge 17 commits from feature/add-default-reminder into main
6 changed files with 109 additions and 1 deletions
Showing only changes of commit 9d2990a23b - Show all commits

View File

@ -0,0 +1,42 @@
const DEFAULT_REMINDER_KEY = 'defaultReminder'
interface DefaultReminderSettings {
konrad marked this conversation as resolved
Review

Reuse type definition. E.g.

{ [type in SavedReminderSettings['type']]: number }
Reuse type definition. E.g. ```ts { [type in SavedReminderSettings['type']]: number } ```
Review

Done.

Done.
enabled: boolean,
amount: number,
}
function calculateDefaultReminderSeconds(type: string, amount: number): number {
switch (type) {
case 'minutes':
return amount * 60
case 'hours':
return amount * 60 * 60
konrad marked this conversation as resolved Outdated

Add as const

Add `as const`

Done.

Done.
case 'days':
return amount * 60 * 60 * 24
case 'months':
konrad marked this conversation as resolved Outdated

We should save the factors in array in a mapping constant. This way the constant can be reused for the test.

We should save the factors in array in a mapping constant. This way the constant can be reused for the test.

Done.

Done.
return amount * 60 * 60 * 24 * 30
}
return 0
}
export function saveDefaultReminder(enabled: boolean, type: string, amount: number) {
const defaultReminderSeconds = calculateDefaultReminderSeconds(type, amount)
localStorage.setItem(DEFAULT_REMINDER_KEY, JSON.stringify(<DefaultReminderSettings>{
enabled,
konrad marked this conversation as resolved
Review

type would better be as keyof typeof AMOUNT_IN_SECONDS.

Maybe we should save the type as new type, e.g.

type IReminderDurationType = 'minutes' | 'hours' | 'days' | 'months'
`type` would better be as `keyof typeof AMOUNT_IN_SECONDS`. Maybe we should save the `type` as new type, e.g. ```ts type IReminderDurationType = 'minutes' | 'hours' | 'days' | 'months' ```
Review

I've changed it to use SavedReminderSettings['type'] instead.

I've changed it to use `SavedReminderSettings['type']` instead.
amount: defaultReminderSeconds,
}))
}
export function getDefaultReminderAmount(): number | null {
const s: string | null = localStorage.getItem(DEFAULT_REMINDER_KEY)
if (s === null) {
return null
}
const settings: DefaultReminderSettings = JSON.parse(s)
return settings.enabled
dpschen marked this conversation as resolved
Review

picky: don't create one-use-variable

picky: don't create one-use-variable
Review

But here I'm using it twice?

But here I'm using it twice?
Review

Beeing too picky is also stupid… you are right!

Beeing too picky is also stupid… you are right!
? settings.amount
: null
}

View File

@ -87,7 +87,11 @@
"language": "Language",
"defaultList": "Default List",
"timezone": "Time Zone",
"overdueTasksRemindersTime": "Overdue tasks reminder email time"
"overdueTasksRemindersTime": "Overdue tasks reminder email time",
"defaultReminder": "Set a default task reminder",
"defaultReminderHint": "If enabled, Vikunja will automatically create a reminder for a task if you set a due date and the task does not have any reminders yet.",
"defaultReminderAmount": "Default task reminder amount",
"defaultReminderAmountHint": "The time difference the reminder will be set to before the due date."
},
"totp": {
"title": "Two Factor Authentication",

View File

@ -12,4 +12,6 @@ export interface IUserSettings extends IAbstract {
weekStart: 0 | 1 | 2 | 3 | 4 | 5 | 6
timezone: string
language: string
defaultReminder: boolean
defaultReminderAmount: number // The amount of seconds a reminder should be set before a given due date
}

View File

@ -13,6 +13,8 @@ export default class UserSettingsModel extends AbstractModel<IUserSettings> impl
weekStart = 0 as IUserSettings['weekStart']
timezone = ''
language = getCurrentLanguage()
defaultReminder = false
konrad marked this conversation as resolved Outdated

se here getDefaultReminderAmount

se here `getDefaultReminderAmount`

Done.

Done.
defaultReminderAmount = 0
constructor(data: Partial<IUserSettings> = {}) {
super()

View File

@ -56,6 +56,10 @@
}
}
.field.has-addons .select select {
konrad marked this conversation as resolved Outdated

Combine with selector below.

Maybe better: use class for specific case in General

Combine with selector below. Maybe better: use class for specific case in General

I've combined them.

I've combined them.
height: 100%;
}
.field.has-addons .control .select select {
height: 100%;
}

View File

@ -18,6 +18,55 @@
</label>
<list-search v-model="defaultList"/>
</div>
<div class="field">
<label class="checkbox">
<input type="checkbox" v-model="settings.defaultReminder"/>
{{ $t('user.settings.general.defaultReminder') }}
</label>
<p class="is-size-7">
{{ $t('user.settings.general.defaultReminderHint') }}
</p>
</div>
<div class="field" v-if="settings.defaultReminder">
<label class="label" for="defaultReminderAmount">
{{ $t('user.settings.general.defaultReminderAmount') }}
</label>
<p class="is-size-7">
{{ $t('user.settings.general.defaultReminderAmountHint') }}
</p>
<div class="field has-addons">
<div class="control">
<input
@keyup.enter="updateSettings"
class="input"
id="defaultReminderAmount"
type="number"
min="0"
v-model="defaultReminderAmount"/>
</div>
konrad marked this conversation as resolved Outdated

In case we don't use vue-i18n pluralisation feature we shouldn't use local messages that can't be precompiled (see: https://vue-i18n.intlify.dev/guide/advanced/optimization.html#optimization).

{{ defaultReminderAmount === 1 ? $t('time.minute') : $t('time.minutes') }}
In case we don't use vue-i18n pluralisation feature we shouldn't use local messages that can't be precompiled (see: https://vue-i18n.intlify.dev/guide/advanced/optimization.html#optimization). ```ts {{ defaultReminderAmount === 1 ? $t('time.minute') : $t('time.minutes') }} ```
<div class="control select">
<select v-model="defaultReminderAmountType">
<option value="minutes">{{
$t('task.repeat.minute' + (defaultReminderAmount === 1 ? '' : 's'))
}}
</option>
<option value="hours">{{
$t('task.repeat.hour' + (defaultReminderAmount === 1 ? '' : 's'))
}}
</option>
<option value="days">{{
$t('task.repeat.day' + (defaultReminderAmount === 1 ? '' : 's'))
}}
</option>
<option value="months">{{
$t('task.repeat.month' + (defaultReminderAmount === 1 ? '' : 's'))
}}
</option>
</select>
</div>
</div>
</div>
<div class="field">
<label class="checkbox">
<input type="checkbox" v-model="settings.overdueTasksRemindersEnabled"/>
@ -175,6 +224,7 @@ import {AuthenticatedHTTPFactory} from '@/http-common'
import {useColorScheme} from '@/composables/useColorScheme'
import {useTitle} from '@/composables/useTitle'
import {objectIsEmpty} from '@/helpers/objectIsEmpty'
import {saveDefaultReminder} from '@/helpers/defaultReminder'
const {t} = useI18n({useScope: 'global'})
useTitle(() => `${t('user.settings.general.title')} - ${t('user.settings.title')}`)
@ -266,9 +316,13 @@ watch(
async function updateSettings() {
localStorage.setItem(playSoundWhenDoneKey, playSoundWhenDone.value ? 'true' : 'false')
setQuickAddMagicMode(quickAddMagicMode.value)
saveDefaultReminder(settings.value.defaultReminder, defaultReminderAmountType.value, defaultReminderAmount.value)
await authStore.saveUserSettings({
settings: {...settings.value},
})
}
konrad marked this conversation as resolved
Review

Shouldn't the fallback be the same as the default in the model?

Shouldn't the fallback be the same as the default in the model?
Review

Actually not, because this is what gets shown in the input when setting this for the first time. I felt like putting a 0 here would feel weired.

Actually not, because this is what gets shown in the input when setting this for the first time. I felt like putting a `0` here would feel weired.
const defaultReminderAmount = ref(1)
const defaultReminderAmountType = ref('days')
</script>