WIP: feat: create KanbanBucket from ListKanban #1571

dpschen · 2022-02-18T12:27:53Z

dpschen commented

2022-02-18 12:27:53 +00:00

By removing the bucket form ListKanban we can reduce the complexity a lot.

~~This is still work-in-progress. Most stuff should work.~~
This might currently not work, but mostly because a small rebase is needed.

The biggest open task is migrating the saveCollapsedBucketState.
=> will solve this in a future pull request

TODO:

Also add these changes: #2634

By removing the bucket form ListKanban we can reduce the complexity a lot. ~~This is still work-in-progress. Most stuff should work.~~ This might currently not work, but mostly because a small rebase is needed. The biggest open task is migrating the saveCollapsedBucketState. => will solve this in a future pull request ## TODO: - [ ] Also add these changes: https://kolaente.dev/vikunja/frontend/pulls/2634

dpschen added the

area/internal-code

label 2022-02-18 12:28:04 +00:00

konrad requested changes 2022-02-19 21:41:45 +00:00

src/components/tasks/partials/KanbanBucket.vue Outdated

						
				@ -0,0 +16,4 @@

				const MIN_SCROLL_HEIGHT_PERCENT = 0.25

				const props = defineProps<{

					bucketIndex: number

konrad commented

2022-02-19 21:33:42 +00:00

number or Number? (What is the difference between them?)

`number` or `Number`? (What is the difference between them?)

✅ 1

dpschen commented

2022-02-20 10:00:01 +00:00

number is the Typescript type of number, while Number is a primitive object wrapper.

See: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#number-string-boolean-symbol-and-object

`number` is the Typescript **type** of number, while `Number` is a [primitive object wrapper](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number). See: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#number-string-boolean-symbol-and-object

👍 1

dpschen marked this conversation as resolved

src/components/tasks/partials/KanbanBucket.vue Outdated

						
				@ -0,0 +17,4 @@

				const props = defineProps<{

					bucketIndex: number

					isCollapsed: boolean

konrad commented

2022-02-19 21:34:57 +00:00

Does this need to be a prop? Since we now have everything bucket-related in a single component, shouldn't it be enough to use a ref?

dpschen commented

2022-02-20 11:31:44 +00:00

Yes and no. Since we currently save the collapsed state of the whole list, when a bucket collapses we need to change that in order to make this bucket specific. I'm still working on this, this is one of the reasosn this is still WIP.

👍 1

dpschen commented

2022-05-10 23:59:26 +00:00

If we want to let's refactor this in a future pull request.
Want to keep this pull request as simple as possible.
Main target was to separate KanbanBucket from ListKanban

If we want to let's refactor this in a future pull request. Want to keep this pull request as simple as possible. Main target was to separate KanbanBucket from ListKanban

dpschen marked this conversation as resolved

src/components/tasks/partials/KanbanBucket.vue Outdated

						
				@ -0,0 +24,4 @@

					dragOptions: Object

					params: Object

					shouldAcceptDrop: boolean

					isDraggingTask: boolean

konrad commented

2022-02-19 21:36:25 +00:00

Does this need to be a prop?

dpschen commented

2022-02-20 11:32:00 +00:00

Will check

dpschen commented

2022-05-10 23:59:53 +00:00

Maybe. But let's go the same way as with #1571 (comment)

Maybe. But let's go the same way as with https://kolaente.dev/vikunja/frontend/pulls/1571#issuecomment-30384

dpschen marked this conversation as resolved

src/components/tasks/partials/KanbanBucket.vue Outdated

						
				@ -0,0 +164,4 @@

				}

				</script>

				<template>

konrad commented

2022-02-19 21:37:30 +00:00

Could you add the <template> before the <script>? Just to keep it consistent.

Could you add the `<template>` before the `<script>`? Just to keep it consistent.

dpschen commented

2022-02-20 10:04:32 +00:00

I was following here the new recommended order. See https://eslint.vuejs.org/rules/component-tags-order.html

When we enable the new settings this will change everywhere.

I was following here the new recommended order. See https://eslint.vuejs.org/rules/component-tags-order.html When we enable the new settings this will change everywhere.

konrad commented

2022-02-20 16:34:26 +00:00

Doesn't the doc you linked say switching <template> and <script> blocks as long as the <style> tag comes last is allowed?

Doesn't the doc you linked say switching `<template>` and `<script>` blocks as long as the `<style>` tag comes last is allowed?

dpschen commented

2022-02-20 17:20:20 +00:00

Fromt reading it seems like it, strange!
Because when I ran #930 this reordering was applied automatically.

I will change it back for this pull request. If #930 changes it we'll handle it there.

Fromt reading it seems like it, strange! Because when I ran https://kolaente.dev/vikunja/frontend/pulls/930 this reordering was applied automatically. I will change it back for this pull request. If https://kolaente.dev/vikunja/frontend/pulls/930 changes it we'll handle it there.

👍 1

dpschen marked this conversation as resolved

src/views/list/ListKanban.vue Outdated

						
				@ -621,1 +315,4 @@

				}

				</style>

				<style lang="scss">

konrad commented

2022-02-19 21:40:36 +00:00

Please merge the two <style> tags since they are the same

Please merge the two `<style>` tags since they are the same

👍 1

dpschen marked this conversation as resolved

src/views/list/ListKanban.vue Outdated

						
				@ -773,2 +370,2 @@

							}

						}

				.new-bucket {

					// Because of reasons, this button ignores the margin we gave it to the right.

konrad commented

2022-02-19 21:41:38 +00:00

Now that I read that comment again, I feel like this might have something to do with margin collapse...

dpschen commented

2022-02-20 10:07:14 +00:00

I will recheck the styles =)

👍 1

dpschen commented

2022-05-11 00:01:10 +00:00

Let's do this in an additional pull request. I want to refactor all these components either way. For now I just wanted to separate the logic. Makes future steps simpler.

dpschen marked this conversation as resolved

dpschen force-pushed feature/feat-kanban-bucket-component from 940938c65a to 4f25e18c38

2022-05-11 00:10:33 +00:00

Compare

dpschen requested review from konrad 2022-05-11 00:10:43 +00:00

dpschen changed title from ~~WIP: feat: create KanbanBucket from ListKanban~~ to feat: create KanbanBucket from ListKanban

2022-05-11 00:10:51 +00:00

dpschen force-pushed feature/feat-kanban-bucket-component from 4f25e18c38 to 7886515193

2022-05-11 01:08:40 +00:00

Compare

frederick commented

2022-05-19 20:00:08 +00:00

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1571-feature-feat-kanban-bucket-compo--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 dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1571-feature-feat-kanban-bucket-compo--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 force-pushed feature/feat-kanban-bucket-component from 7e963d3c8e to dfe3356df3

2022-05-19 20:14:28 +00:00

Compare

dpschen commented

2022-05-21 15:55:35 +00:00

This is currently out of date, but was working already.
I'm not sure if I should update this if we don't merge it then?

This is currently out of date, but was working already. I'm not sure if I should update this if we don't merge it then?

konrad commented

2022-05-22 15:52:26 +00:00

I think this would be a good thing to have. If you want to continue working on this, I'd appreciate a rebase.

👍 1

dpschen referenced this pull request

2022-09-30 11:37:27 +00:00

Done bucket behaviour #1796

dpschen force-pushed feature/feat-kanban-bucket-component from dfe3356df3 to 244b5318c1

2022-10-27 14:40:41 +00:00

Compare

dpschen force-pushed feature/feat-kanban-bucket-component from 244b5318c1 to 0750839e64

2022-10-27 14:51:03 +00:00

Compare

dpschen force-pushed feature/feat-kanban-bucket-component from 0750839e64 to d6970bdd6d

2022-10-27 14:53:54 +00:00

Compare

konrad commented

2022-10-27 19:59:09 +00:00

Trying to do anything on the kanban board returns this error:

Trying to do anything on the kanban board returns this error: ![image](/attachments/f9d56908-0c1a-4423-ac21-30cf12e60f02)

image.png

4.4 KiB

👍 1

dpschen changed title from ~~feat: create KanbanBucket from ListKanban~~ to WIP: feat: create KanbanBucket from ListKanban

2022-10-28 11:58:51 +00:00

dpschen commented

2022-10-28 11:59:39 +00:00

Trying to do anything on the kanban board returns this error:

Yes, sry this was working once, but I need to make it work again. I rebased it to get a better understanding of the diff. But it's not yet functional again.

> Trying to do anything on the kanban board returns this error: > > ![image](/attachments/f9d56908-0c1a-4423-ac21-30cf12e60f02) Yes, sry this was working once, but I need to make it work again. I rebased it to get a better understanding of the diff. But it's not yet functional again.

dpschen referenced this pull request

2022-11-07 11:15:48 +00:00

feat: ListKanban script setup #2643

dpschen referenced this pull request

2022-11-07 14:29:44 +00:00

feat: ListKanban script setup #2643

konrad commented

2023-10-22 17:41:49 +00:00

Closing this as it appears stale.

Please rebase and reopen if you think this is still relevant.

Closing this as it appears stale. Please rebase and reopen if you think this is still relevant.

konrad closed this pull request