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

feat: task checklist improvements #797

Merged
konrad merged 5 commits from feature/checklist-improvements into main 2021-09-29 18:31:15 +00:00
Conversation 0 Commits 5 Files Changed 9 +232 -37
konrad commented 2021-09-26 20:21:16 +00:00
Owner
Copy Link

This PR adds checklist indicators in task detail and list views:

image

image

Based on this discussion in the forum.

This PR adds checklist indicators in task detail and list views: ![image](/attachments/18796aaa-e8e8-4730-a52a-9a6741791132) ![image](/attachments/4a0a996a-16ac-446d-a90e-156b88d9f4da) Based on [this discussion in the forum](https://community.vikunja.io/t/lists-but-not-lists-of-tasks/225/6).
image.png
9.9 KiB
image.png
21 KiB
konrad added 2 commits 2021-09-26 20:21:16 +00:00
continuous-integration/drone/pr Build is passing Details
0c24ef6166
feat: show checklist summary on tasks
konrad changed title from Task checklist improvements to feat: task checklist improvements 2021-09-26 20:41:43 +00:00
dpschen reviewed 2021-09-26 22:40:02 +00:00
src/components/tasks/partials/checklist-summary.vue Outdated
@ -0,0 +43,4 @@
display: inline-flex;
align-items: center;
> svg {
dpschen commented 2021-09-26 22:40:02 +00:00
Member

No need to nest svg and circle in scoped styles.

No need to nest `svg` and `circle` in scoped styles.
konrad commented 2021-09-27 19:15:10 +00:00
Author
Owner

Fixed.

Fixed.
👍 1
konrad marked this conversation as resolved
dpschen reviewed 2021-09-27 11:18:23 +00:00
src/helpers/checklistFromText.ts Outdated
@ -0,0 +19,4 @@
// 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 => {
dpschen commented 2021-09-27 11:18:23 +00:00
Member

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

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 2021-09-27 19:18:13 +00:00
Author
Owner

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.
konrad marked this conversation as resolved
dpschen reviewed 2021-09-27 11:21:58 +00:00
src/helpers/checklistFromText.ts Outdated
@ -0,0 +43,4 @@
checked: number
}
export const getChecklistStatistics = (text: string): CheckboxStatistics => {
dpschen commented 2021-09-27 11:21:58 +00:00
Member

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.

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 2021-09-27 19:21:00 +00:00
Author
Owner

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 2021-09-28 07:18:16 +00:00
Member

Looks good :)

Looks good :)
konrad marked this conversation as resolved
konrad added 2 commits 2021-09-27 19:14:44 +00:00
dpschen reviewed 2021-09-28 07:15:50 +00:00
src/helpers/checklistFromText.ts
@ -0,0 +1,56 @@
const searchLength = 6
dpschen commented 2021-09-28 07:15:50 +00:00
Member

searchLength, listPrefixes and unchecked are not used anymore. rest of this file looks good :) cool!

`searchLength`, `listPrefixes` and `unchecked` are not used anymore. rest of this file looks good :) cool!
konrad commented 2021-09-29 18:14:35 +00:00
Author
Owner

Good catch :) I've removed them.

Good catch :) I've removed them.
konrad marked this conversation as resolved
konrad added 1 commit 2021-09-29 18:14:23 +00:00
continuous-integration/drone/pr Build is passing Details
9cc37bcaac
chore: cleanup unused variables
konrad merged commit 96523f1fbf into main 2021-09-29 18:31:15 +00:00
konrad deleted branch feature/checklist-improvements 2021-09-29 18:31:15 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#797
No description provided.
Delete Branch "feature/checklist-improvements"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?