vikunja/vikunja
5
48
Fork 86
Code Issues 150 Pull Requests 37 Packages Releases 37 Activity

feat: add generic types to multiselect #2618

Open
dpschen wants to merge 1 commits from dpschen/vikunja:feat/multiselect-generic-types into main
pull from: dpschen/vikunja:feat/multiselect-generic-types
merge into: vikunja:main
Conversation 2 Commits 1 Files Changed 1 +11 -20
dpschen commented 2024-07-17 16:01:00 +00:00
Member
Copy Link

This improves (!) the types of Multiselect — it doesn't fix them

This improves (!) the types of Multiselect — it doesn't fix them
dpschen added 1 commit 2024-07-17 16:01:06 +00:00
feat: add generic types to multiselect
Some checks failed
continuous-integration/drone/pr Build is failing
Details
76b5e1a9f2 
This improves (!) the types of Multiselect — it doesn't fix them
dpschen force-pushed feat/multiselect-generic-types from 76b5e1a9f2 to 0582cdbd9c 2024-07-17 17:31:47 +00:00 Compare
frederick commented 2024-07-17 17:44:23 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2618-feat-multiselect-generic-types--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 somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2618-feat-multiselect-generic-types--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 somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
dpschen changed title from WIP: feat: add generic types to multiselect to feat: add generic types to multiselect 2024-07-17 20:43:04 +00:00
dpschen requested review from konrad 2024-07-17 20:43:11 +00:00
dpschen commented 2024-07-18 10:28:40 +00:00
Author
Member
Copy Link

I want to replace this component with one from radix-vue as fast as possible

I want to replace this component with one from radix-vue as fast as possible
konrad reviewed 2024-07-19 07:36:35 +00:00
frontend/src/components/input/Multiselect.vue
@ -221,3 +215,3 @@
const showSearchResults = ref(false)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const internalValue = ref<string | { [key: string]: any } | any[] | null>(null)
const internalValue = ref<string | T | any[] | null>(null)
konrad commented 2024-07-19 07:36:20 +00:00
Owner
Copy Link

Shouldn't this be T[] as well?

Shouldn't this be `T[]` as well?
dpschen commented 2024-07-19 09:29:17 +00:00
Author
Member
Copy Link

Maybe? If you're sure I can replace it.

That would mean that the original type was wrong (should have been { [key: string]: any }[]). I was also confused by the any[]

To be honest I only replaced { [key: string]: any } with T in this PR.

Maybe? If you're sure I can replace it. That would mean that the original type was wrong (should have been `{ [key: string]: any }[]`). I was also confused by the `any[]` To be honest I only replaced `{ [key: string]: any }` with T in this PR.
konrad commented 2024-07-19 09:47:19 +00:00
Owner
Copy Link

It could be that any[] here should be T[] and the single T could stay. I'm not sure, just something I wondered.

It could be that `any[]` here should be `T[]` and the single `T` could stay. I'm not sure, just something I wondered.
😆 1
dpschen commented 2024-07-19 10:03:41 +00:00
Author
Member
Copy Link

I will check it out again and turn my brain on. Maybe I can find it out for real :D

I will check it out again and turn my brain on. Maybe I can find it out for real :D
🚀 1
All checks were successful
continuous-integration/drone/pr Build is passing
Required
Details
This pull request doesn't have enough required approvals yet. 0 of 1 approvals granted from users or teams on the allowlist.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u feat/multiselect-generic-types:dpschen-feat/multiselect-generic-types
git checkout dpschen-feat/multiselect-generic-types
Sign in to join this conversation.
Reviewers
No Reviewers
konrad
Labels
Clear labels
dependencies
duplicate
This issue or pull request already exists
help wanted
Need some help
invalid
Something is wrong
kind/bug
Something is not working
kind/feature
New feature
needs reproduction
question
More information is needed
security
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vikunja/vikunja#2618
No description provided.
Delete Branch "dpschen/vikunja:feat/multiselect-generic-types"

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?