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 +11 -9
3 changed files with 11 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5fb45afb12 - Show all commits

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

@ -14,7 +14,7 @@
v-model="periodInput.duration.days"
type="number"
min="0"
/> {{ $t('task.reminder.daysShort') }}
/> {{ $t('task.reminder.days') }}
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"
class="input"
@ -166,7 +166,7 @@ function formatDuration(reminderPeriod: number): string {
return '00:00'
}
const duration = secondsToPeriod(Math.abs(reminderPeriod))
return (duration.days > 0 ? duration.days + ' d ' : '') +
return (duration.days > 0 ? `${duration.days} ${t('task.reminder.days')} `: '') +
('' + duration.hours).padStart(2, '0') + ':' +
('' + duration.minutes).padStart(2, '0')
}

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

@ -3,12 +3,13 @@ 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): {days: number, hours: number, minutes: number, seconds: number} {
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}
export function secondsToPeriod(seconds: number): {days: number, hours: number, minutes: number, seconds: number} {
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
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),
}
}
/**

3
src/i18n/lang/en.json
View File

@ -723,7 +723,8 @@
"reminder": {
"hoursShort": "HH",
"minutesShort": "MM",
"daysShort": "d"
"daysShort": "d",
"days": "days"
},
"repeat": {
"everyDay": "Every Day",