feat: automatically create subtask relations based on indention #2443

konrad · 2022-09-28T17:48:15Z

konrad commented

2022-09-28 17:48:15 +00:00

This PR adds the automatic creation of subtasks via indention.

For example, creating tasks with this multiline input:

parent task
  sub task one
  sub task two
    sub task three

will make sub task one and sub task two a sub task of parent task and sub task three a sub task of sub task two.

This PR adds the automatic creation of subtasks via indention. For example, creating tasks with this multiline input: ``` parent task sub task one sub task two sub task three ``` will make `sub task one` and `sub task two` a sub task of `parent task` and `sub task three` a sub task of `sub task two`.

🎉 1

konrad requested review from dpschen 2022-09-28 17:48:20 +00:00

frederick commented

2022-09-28 17:53:01 +00:00

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://2443-feature-subtask-via-indention--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi konrad! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://2443-feature-subtask-via-indention--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.

dpschen requested changes 2022-09-28 19:14:57 +00:00

src/components/tasks/add-task.vue Outdated

						
				@ -165,1 +164,3 @@

						if (title === '') {

					const createdTasks: ITask[] = []

					const tasksToCreate = parseSubtasksViaIndention(newTaskTitle.value)

					const newTasks = tasksToCreate.map(async t => {

dpschen commented

2022-09-28 18:36:59 +00:00

spread title: ({title}) => {

spread title: `({title}) => {`

konrad marked this conversation as resolved

src/components/tasks/add-task.vue Outdated

						
				@ -181,0 +185,4 @@

						const relations = tasksToCreate.map(async t => {

							const createdTask = createdTasks.find(ct => ct.title === t.title)

							if (t.parent === null) {

								emit('taskAdded', createdTask)

dpschen commented

2022-09-28 19:02:25 +00:00

In line 194 we return if typeof createdTask === 'undefined'.
This implies that createdTask could be undefined here aswell. Is that intended?

In line 194 we return if `typeof createdTask === 'undefined'`. This implies that createdTask could be undefined here aswell. Is that intended?

konrad commented

2022-09-29 16:05:42 +00:00

mhh I didn't think this through. It should never be null.

I've adjusted this so that there's a check before emitting it.

mhh I didn't think this through. It should never be null. I've adjusted this so that there's a check before emitting it.

👍 1

konrad marked this conversation as resolved

src/helpers/parseSubtasksViaIndention.ts Outdated

						
				@ -0,0 +1,42 @@

				export interface TaskWithParent {

dpschen commented

2022-09-28 19:11:16 +00:00

Why do we create a new interface for this and don't use ITask for this?

konrad commented

2022-09-29 16:07:18 +00:00

Because this is only used to hold the title and parent title. Using ITask would probably work but I felt it would introduce too much overhead for a simple relation.

Because this is only used to hold the title and parent title. Using `ITask` would probably work but I felt it would introduce too much overhead for a simple relation.

dpschen commented

2022-09-29 18:15:58 +00:00

Unsure: would it make sense to create the new type still on ITask with the help of Pick, like:

export interface TaskWithPartent = Pick<ITask, 'id' | 'parent'>

Unsure: would it make sense to create the new type still on `ITask` with the help of `Pick`, like: ```ts export interface TaskWithPartent = Pick<ITask, 'id' | 'parent'> ```

konrad commented

2022-09-30 11:36:02 +00:00

I think explicitely declaring it is more readable.

dpschen commented

2022-09-30 14:11:45 +00:00

Good with me. I still think we should reuse the types from the task interface. Meaning:

export interface TaskWithParent {
	title: string,
	parent: ITask['id'] | null,
}

The reason beeing here: In the medium term we should change the id in the frontend to a string to make indexing easier. Because objects don't support number types as keys. The alternative would beto use Maps everywhere which is unrealistic (because they do support numbers as keys). By using zod this shouldn't be too complex.

Good with me. I still think we should reuse the types from the task interface. Meaning: ```t export interface TaskWithParent { title: string, parent: ITask['id'] | null, } ``` The reason beeing here: In the medium term we should change the id in the frontend to a string to make indexing easier. Because objects don't support number types as keys. The alternative would beto use Maps everywhere which is unrealistic (because they do support numbers as keys). By using zod this shouldn't be too complex.

konrad commented

2022-09-30 16:40:26 +00:00

But we're not even using IDs here, only string titles.

dpschen commented

2022-09-30 17:11:15 +00:00

Arrrrgh… makes sense. Sry i missed that part :D

dpschen marked this conversation as resolved

src/helpers/parseSubtasksViaIndention.ts Outdated

						
				@ -0,0 +14,4 @@

				export function parseSubtasksViaIndention(taskTitles: string): TaskWithParent[] {

					const titles = taskTitles.split(/[\r\n]+/)

					return titles.map((t, i) => {

dpschen commented

2022-09-28 19:09:18 +00:00

Picky: use clear variable name, e.g. title, index.

Makes reading easier since you don't have to remember and jump to the var definition.

Picky: use clear variable name, e.g. `title`, `index`. Makes reading easier since you don't have to remember and jump to the var definition.

konrad commented

2022-09-29 16:09:46 +00:00

Makes sense. Done!

konrad marked this conversation as resolved

src/helpers/parseSubtasksViaIndention.ts Outdated

						
				@ -0,0 +20,4 @@

							parent: null,

						}

						const matched = spaceRegex.exec(t)

dpschen commented

2022-09-28 19:13:21 +00:00

Move matched and matchedSpaces in condition aswell since it's not used for index === 0

Move `matched` and `matchedSpaces` in condition aswell since it's not used for `index === 0`

konrad commented

2022-09-29 16:10:08 +00:00

Added an earlier break for index === 0.

Added an earlier break for `index === 0`.

👍 1

konrad marked this conversation as resolved

konrad was assigned by dpschen

2022-09-28 19:15:03 +00:00

konrad force-pushed feature/subtask-via-indention from c5f4f22923 to 65b1d9abc5

2022-09-29 16:02:52 +00:00

Compare

dpschen reviewed 2022-09-29 18:18:00 +00:00

dpschen reviewed 2022-09-29 18:21:19 +00:00

src/helpers/parseSubtasksViaIndention.ts Outdated

						
				@ -0,0 +9,4 @@

				const spaceRegex = /^ */

				// taskTitles should be multiple lines of task tiles with indention to declare their parent/subtask