feature/feat-useList-composable #2589
No reviewers
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
No Assignees
3 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#2589
Loading…
Reference in New Issue
No description provided.
Delete Branch "dpschen/frontend:feature/feat-useList-composable"
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?
e75f00744d
tob715cf4d70
b715cf4d70
to54564271fe
Hi dpschen!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://2589-feature-feat-uselist-composable--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!
WIP: feature/feat-useList-composableto feature/feat-useList-composable@ -0,0 +1,195 @@
<template>
I still have to delete this.
Done
@ -169,7 +169,6 @@
"title": "List Title",
"color": "Color",
"lists": "Lists",
"list": "List",
this entry is beeing overwritten below
@ -328,3 +297,3 @@
return response.data.map(entry => this.modelGetAllFactory(entry))
}
return this.modelGetAllFactory(response.data)
return [this.modelGetAllFactory(response.data)]
this seemed wrong. when wouldn't you receive an array here?
I think there really is no case where no array is returned.
Maybe it would be better to throw instead. Because this fucks up the return types. That's why I added the brackets.
Fixed
@ -45,0 +47,4 @@
return
}
const cancel = this.setLoading()
unsure: maybe I should remove the setLoading here since the list shouldn't indicate a loading just for the background?
I think that would make sense.
Fixed
@ -136,8 +94,6 @@ export const useBaseStore = defineStore('base', () => {
loading: readonly(loading),
ready: readonly(ready),
currentList: readonly(currentList),
background: readonly(background),
I might need to readd this. I didn't realize that the info if a background is loaded is necessary.
@ -95,2 +104,4 @@
}
async function loadList(listId: List['id']) {
const cancel = setModuleLoading(this, setIsLoading)
remove 'this'
Was already fixed
@ -162,0 +216,4 @@
namespaceStore.addListToNamespace(duplicate.list)
setList(duplicate.list)
success({message: i18n.global.t('list.duplicate.success')})
router.push({name: 'list.index', params: {listId: duplicate.list.id}})
in case of the modal, where the modal has a view we should replace the view instead. maybe always?
Doesn't the view in
list.index
do that?Yes it does. But we still use
router.push()
. What I meant was usingrouter.replace()
instead, because you wouldn't want to go back to the route of the dialog.For routes like the task detail popup we should have the option to go back to the route with a popup. Maybe for settings as well.
Yes, maybe this should be configurable via a prop
Keeping this as it is for now. This is something that should be tackled in the modal or even in the dialog branch later.
@ -162,0 +222,4 @@
const backgroundService = shallowReactive(new BackgroundUnsplashService())
async function setUnsplashBackground(listId: IList['id'], backgroundId: IBackgroundImage['id']) {
// Don't set a background if we're in the process of setting one
why not?
I think this was to prevent multiple clicks quickly after each other. The better solution would of course be disabling the button.
Okay, will do that instead.
@ -162,0 +227,4 @@
return
}
const list: IList = await backgroundService.update({
fix: type should be returned by update.
Fixed
@ -211,0 +457,4 @@
watch(
() => [listVal.value.id, listVal.value.backgroundBlurHash] as [IList['id'], IList['backgroundBlurHash']],
async ([listId, blurHash], oldValue) => {
if (backgroundLoading.value) {
I copied this from the old code, but I think that we shouldn't return in that case
@ -211,0 +475,4 @@
await listStore.loadListBackground(
listVal.value,
(value) => {
blurHashUrl.value = value
if the list or background has changed in between we shouldn't set this.
When could that be the case?
If you click on one list that has a background and then on another that has none. The promise would still finish and then set the (wrong) background.
Makes sense!
fad87f12e0
tof80be88cb3
f80be88cb3
to86184a4f9f
Is this done?
86184a4f9f
to7e6aa0a9f4
7e6aa0a9f4
to531b0baf02
531b0baf02
to5fbd4c80cb
Closing this as it appears stale.
Please rebase and reopen if you think this is still relevant.