feat: task checklist improvements #797
|
@ -53,6 +53,7 @@ import hljs from 'highlight.js/lib/common'
|
|||
|
||||
import AttachmentModel from '../../models/attachment'
|
||||
import AttachmentService from '../../services/attachment'
|
||||
import {findCheckboxesInText} from '../../helpers/checklistFromText'
|
||||
|
||||
export default {
|
||||
name: 'editor',
|
||||
|
@ -303,36 +304,7 @@ export default {
|
|||
return str.substr(0, index) + replacement + str.substr(index + replacement.length)
|
||||
},
|
||||
findNthIndex(str, n) {
|
||||
|
||||
const searchLength = 6
|
||||
const listPrefixes = ['*', '-']
|
||||
|
||||
let inChecked, inUnchecked, startIndex = 0
|
||||
// We're building an array with all checkboxes, checked or unchecked.
|
||||
// I've found this to be the best way to always get the results I need.
|
||||
// The difficulty without an index is that we need to get all checkboxes, checked and unchecked
|
||||
// and calculate our index based off that to compare it and find the checkbox we need.
|
||||
let checkboxes = []
|
||||
|
||||
// Searching in two different loops for each search term since that is way easier and more predicatble
|
||||
// More "intelligent" solutions sometimes don't have all values or duplicates.
|
||||
// Because we're sorting and removing duplicates of them, we can safely put everything in one giant array.
|
||||
listPrefixes.forEach(pref => {
|
||||
while ((inChecked = str.indexOf(`${pref} [x]`, startIndex)) > -1) {
|
||||
checkboxes.push(inChecked)
|
||||
startIndex = startIndex + searchLength
|
||||
}
|
||||
|
||||
startIndex = 0
|
||||
while ((inUnchecked = str.indexOf(`${pref} [ ]`, startIndex)) > -1) {
|
||||
checkboxes.push(inUnchecked)
|
||||
startIndex = startIndex + searchLength
|
||||
}
|
||||
})
|
||||
|
||||
checkboxes.sort((a, b) => a - b)
|
||||
checkboxes = checkboxes.filter((v, i, s) => s.indexOf(v) === i && v > -1)
|
||||
|
||||
const checkboxes = findCheckboxesInText(str)
|
||||
return checkboxes[n]
|
||||
},
|
||||
renderPreview() {
|
||||
|
@ -435,7 +407,7 @@ export default {
|
|||
console.debug(index, this.text.substr(index, 9))
|
||||
|
||||
const listPrefix = this.text.substr(index, 1)
|
||||
|
||||
|
||||
if (checked) {
|
||||
this.text = this.replaceAt(this.text, index, `${listPrefix} [x] `)
|
||||
} else {
|
||||
|
@ -475,7 +447,7 @@ export default {
|
|||
input[type="checkbox"] {
|
||||
margin-right: .5rem;
|
||||
}
|
||||
|
||||
|
||||
&.has-checkbox {
|
||||
margin-left: -2em;
|
||||
list-style: none;
|
||||
|
|
|
@ -0,0 +1,60 @@
|
|||
<template>
|
||||
<span v-if="checklist.total > 0" class="checklist-summary">
|
||||
<svg width="12" height="12">
|
||||
<circle stroke-width="2" fill="transparent" cx="50%" cy="50%" r="5"></circle>
|
||||
<circle stroke-width="2" stroke-dasharray="31" :stroke-dashoffset="checklistCircleDone"
|
||||
stroke-linecap="round" fill="transparent" cx="50%" cy="50%" r="5"></circle>
|
||||
</svg>
|
||||
<span>
|
||||
{{ $t(checklist.total === checklist.checked ? 'task.checklistAllDone' : 'task.checklistTotal', checklist) }}
|
||||
</span>
|
||||
</span>
|
||||
</template>
|
||||
|
||||
<script>
|
||||
import {getChecklistStatistics} from '../../../helpers/checklistFromText'
|
||||
|
||||
export default {
|
||||
name: 'checklist-summary',
|
||||
props: {
|
||||
task: {
|
||||
required: true,
|
||||
},
|
||||
},
|
||||
computed: {
|
||||
checklist() {
|
||||
return getChecklistStatistics(this.task.description)
|
||||
},
|
||||
checklistCircleDone() {
|
||||
const r = 5
|
||||
const c = Math.PI * (r * 2)
|
||||
|
||||
const progress = this.checklist.checked / this.checklist.total * 100
|
||||
|
||||
return ((100 - progress) / 100) * c
|
||||
},
|
||||
},
|
||||
}
|
||||
</script>
|
||||
|
||||
<style scoped lang="scss">
|
||||
.checklist-summary {
|
||||
color: $grey-500;
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
|
||||
svg {
|
||||
konrad marked this conversation as resolved
Outdated
|
||||
transform: rotate(-90deg);
|
||||
transition: stroke-dashoffset 0.35s;
|
||||
margin-right: .25rem;
|
||||
|
||||
circle {
|
||||
stroke: $grey-400;
|
||||
|
||||
&:last-child {
|
||||
stroke: $primary;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
</style>
|
|
@ -59,6 +59,7 @@
|
|||
<icon icon="align-left"/>
|
||||
</span>
|
||||
</span>
|
||||
<checklist-summary :task="task"/>
|
||||
</router-link>
|
||||
<progress
|
||||
class="progress is-small"
|
||||
|
@ -94,6 +95,7 @@ import Fancycheckbox from '../../input/fancycheckbox'
|
|||
import DeferTask from './defer-task'
|
||||
import {closeWhenClickedOutside} from '@/helpers/closeWhenClickedOutside'
|
||||
import {playPop} from '@/helpers/playPop'
|
||||
import ChecklistSummary from './checklist-summary'
|
||||
|
||||
export default {
|
||||
name: 'singleTaskInList',
|
||||
|
@ -105,6 +107,7 @@ export default {
|
|||
}
|
||||
},
|
||||
components: {
|
||||
ChecklistSummary,
|
||||
DeferTask,
|
||||
Fancycheckbox,
|
||||
User,
|
||||
|
@ -172,10 +175,10 @@ export default {
|
|||
this.task = t
|
||||
this.$emit('task-updated', t)
|
||||
this.$message.success({
|
||||
message: this.task.done ?
|
||||
this.$t('task.doneSuccess') :
|
||||
this.$t('task.undoneSuccess'),
|
||||
}, [{
|
||||
message: this.task.done ?
|
||||
this.$t('task.doneSuccess') :
|
||||
this.$t('task.undoneSuccess'),
|
||||
}, [{
|
||||
title: 'Undo',
|
||||
callback: () => {
|
||||
this.task.done = !this.task.done
|
||||
|
|
|
@ -0,0 +1,91 @@
|
|||
import {findCheckboxesInText, getChecklistStatistics} from './checklistFromText'
|
||||
|
||||
describe('Find checklists in text', () => {
|
||||
it('should find no checkbox', () => {
|
||||
const text: string = 'Lorem Ipsum'
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(0)
|
||||
})
|
||||
it('should find multiple checkboxes', () => {
|
||||
const text: string = `* [ ] Lorem Ipsum
|
||||
* [ ] Dolor sit amet
|
||||
|
||||
Here's some text in between
|
||||
|
||||
* [x] Dolor sit amet
|
||||
- [ ] Dolor sit amet`
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(4)
|
||||
expect(checkboxes[0]).toBe(0)
|
||||
expect(checkboxes[1]).toBe(18)
|
||||
expect(checkboxes[2]).toBe(69)
|
||||
})
|
||||
it('should find one checkbox with *', () => {
|
||||
const text: string = '* [ ] Lorem Ipsum'
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(1)
|
||||
expect(checkboxes[0]).toBe(0)
|
||||
})
|
||||
it('should find one checkbox with -', () => {
|
||||
const text: string = '- [ ] Lorem Ipsum'
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(1)
|
||||
expect(checkboxes[0]).toBe(0)
|
||||
})
|
||||
it('should find one checked checkbox with *', () => {
|
||||
const text: string = '* [x] Lorem Ipsum'
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(1)
|
||||
expect(checkboxes[0]).toBe(0)
|
||||
})
|
||||
it('should find one checked checkbox with -', () => {
|
||||
const text: string = '- [x] Lorem Ipsum'
|
||||
const checkboxes = findCheckboxesInText(text)
|
||||
|
||||
expect(checkboxes).toHaveLength(1)
|
||||
expect(checkboxes[0]).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Get Checklist Statistics in a Text', () => {
|
||||
it('should find no checkbox', () => {
|
||||
const text: string = 'Lorem Ipsum'
|
||||
const stats = getChecklistStatistics(text)
|
||||
|
||||
expect(stats.total).toBe(0)
|
||||
})
|
||||
it('should find one checkbox', () => {
|
||||
const text: string = '* [ ] Lorem Ipsum'
|
||||
const stats = getChecklistStatistics(text)
|
||||
|
||||
expect(stats.total).toBe(1)
|
||||
expect(stats.checked).toBe(0)
|
||||
})
|
||||
it('should find one checked checkbox', () => {
|
||||
const text: string = '* [x] Lorem Ipsum'
|
||||
const stats = getChecklistStatistics(text)
|
||||
|
||||
expect(stats.total).toBe(1)
|
||||
expect(stats.checked).toBe(1)
|
||||
})
|
||||
it('should find multiple mixed and matched', () => {
|
||||
const text: string = `* [ ] Lorem Ipsum
|
||||
* [ ] Dolor sit amet
|
||||
* [x] Dolor sit amet
|
||||
- [x] Dolor sit amet
|
||||
|
||||
Here's some text in between
|
||||
|
||||
* [x] Dolor sit amet
|
||||
- [ ] Dolor sit amet`
|
||||
const stats = getChecklistStatistics(text)
|
||||
|
||||
expect(stats.total).toBe(6)
|
||||
expect(stats.checked).toBe(3)
|
||||
})
|
||||
})
|
|
@ -0,0 +1,53 @@
|
|||
const checked = '[x]'
|
||||
konrad marked this conversation as resolved
dpschen
commented
`searchLength`, `listPrefixes` and `unchecked` are not used anymore. rest of this file looks good :) cool!
konrad
commented
Good catch :) I've removed them. Good catch :) I've removed them.
|
||||
|
||||
interface CheckboxStatistics {
|
||||
total: number
|
||||
checked: number
|
||||
}
|
||||
|
||||
interface MatchedCheckboxes {
|
||||
checked: number[]
|
||||
unchecked: number[]
|
||||
}
|
||||
|
||||
const getCheckboxesInText = (text: string): MatchedCheckboxes => {
|
||||
const regex = /[*-] \[[ x]]/g
|
||||
let match
|
||||
const checkboxes: MatchedCheckboxes = {
|
||||
checked: [],
|
||||
unchecked: [],
|
||||
}
|
||||
|
||||
while ((match = regex.exec(text)) !== null) {
|
||||
if (match[0].endsWith(checked)) {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
I might be wrong because of poor regex logic knowledge, maybe it's possible to use a simple one here: In combination with https://stackoverflow.com/a/2295681/15522256 a single loop + analysing the found match would be enough. That said: Ignore if comment is annoying: I might be wrong because of poor regex logic knowledge, maybe it's possible to use a simple one here:
https://regex101.com/r/WuD5uS/1
In combination with https://stackoverflow.com/a/2295681/15522256 a single loop + analysing the found match would be enough.
---
That said: Ignore if comment is annoying:
I'm forcefully trying to find a good usecase for regex, learn something new and am aware that the current solution is more than sufficient
konrad
commented
I've modified it now to use a regex. The original implementation from the editor component was from a time where I didn't really knew regex, I think using one here makes it a lot simpler and easier to read. I've modified it now to use a regex. The original implementation from the editor component was from a time where I didn't really knew regex, I think using one here makes it a lot simpler and easier to read.
|
||||
checkboxes.checked.push(match.index)
|
||||
} else {
|
||||
checkboxes.unchecked.push(match.index)
|
||||
}
|
||||
}
|
||||
|
||||
return checkboxes
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the indices where checkboxes start and end in the given text.
|
||||
*
|
||||
* @param text
|
||||
*/
|
||||
export const findCheckboxesInText = (text: string): number[] => {
|
||||
const checkboxes = getCheckboxesInText(text)
|
||||
|
||||
return [
|
||||
...checkboxes.checked,
|
||||
...checkboxes.unchecked,
|
||||
].sort()
|
||||
}
|
||||
|
||||
export const getChecklistStatistics = (text: string): CheckboxStatistics => {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Independant of the regex: while looping through the loop we already know if a checkbox is checked or not. Why not save that info and return it by default. If you don't want to return an object in the original function you could write a second one that wraps the loop and omits all other info than Independant of the regex: while looping through the loop we already know if a checkbox is checked or not. Why not save that info and return it by default. If you don't want to return an object in the original function you could write a second one that wraps the loop and omits all other info than `checkboxes`.
konrad
commented
I've modified that as well and moved the logic into another (unexported) function which returns the checked and unchecked independently from each other. I'm then using that function in the two other ones. The original implementation in the editor component came out of a lot of trial and error and, because I didn't have any tests at that time either, was at the end a "do not touch, magic" kind of function. I don't even recall why I had two loops and then weren't even using the result. This new version is a lot cleaner. I've modified that as well and moved the logic into another (unexported) function which returns the checked and unchecked independently from each other. I'm then using that function in the two other ones.
The original implementation in the editor component came out of a lot of trial and error and, because I didn't have any tests at that time either, was at the end a "do not touch, magic" kind of function. I don't even recall why I had two loops and then weren't even using the result. This new version is a lot cleaner.
dpschen
commented
Looks good :) Looks good :)
|
||||
const checkboxes = getCheckboxesInText(text)
|
||||
|
||||
return {
|
||||
total: checkboxes.checked.length + checkboxes.unchecked.length,
|
||||
checked: checkboxes.checked.length,
|
||||
}
|
||||
}
|
|
@ -493,6 +493,8 @@
|
|||
"doneSuccess": "The task was successfully marked as done.",
|
||||
"undoneSuccess": "The task was successfully un-marked as done.",
|
||||
"openDetail": "Open task detail view",
|
||||
"checklistTotal": "{checked} of {total} tasks",
|
||||
"checklistAllDone": "{total} tasks",
|
||||
"show": {
|
||||
"titleCurrent": "Current Tasks",
|
||||
"titleDates": "Tasks from {from} until {to}",
|
||||
|
|
|
@ -10,6 +10,7 @@
|
|||
|
||||
.subtitle {
|
||||
color: $grey-500;
|
||||
margin-bottom: 1rem;
|
||||
|
||||
a {
|
||||
color: $grey-800;
|
||||
|
@ -178,6 +179,10 @@
|
|||
color: $grey-500;
|
||||
text-align: right;
|
||||
}
|
||||
|
||||
.checklist-summary {
|
||||
margin-left: .25rem;
|
||||
}
|
||||
}
|
||||
|
||||
.link-share-container:not(.has-background) .task-view {
|
||||
|
|
|
@ -195,6 +195,11 @@
|
|||
border-bottom-color: $grey-300;
|
||||
}
|
||||
}
|
||||
|
||||
.checklist-summary {
|
||||
padding-left: .5rem;
|
||||
font-size: .9rem;
|
||||
}
|
||||
|
||||
.progress {
|
||||
width: 50px;
|
||||
|
|
|
@ -8,9 +8,11 @@
|
|||
{{ getListTitle(parent.list) }}
|
||||
</router-link>
|
||||
</h6>
|
||||
|
||||
<checklist-summary :task="task"/>
|
||||
|
||||
<!-- Content and buttons -->
|
||||
<div class="columns">
|
||||
<div class="columns mt-2">
|
||||
<!-- Content -->
|
||||
<div :class="{'is-two-thirds': canWrite}" class="column">
|
||||
<div class="columns details">
|
||||
|
@ -445,10 +447,12 @@ import TaskSubscription from '@/components/misc/subscription.vue'
|
|||
import {CURRENT_LIST} from '@/store/mutation-types'
|
||||
|
||||
import {uploadFile} from '@/helpers/attachments'
|
||||
import ChecklistSummary from '../../components/tasks/partials/checklist-summary'
|
||||
|
||||
export default {
|
||||
name: 'TaskDetailView',
|
||||
components: {
|
||||
ChecklistSummary,
|
||||
TaskSubscription,
|
||||
Datepicker,
|
||||
ColorPicker,
|
||||
|
|
No need to nest
svg
andcircle
in scoped styles.Fixed.