feat(task): cover image for tasks #2460
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#2460
Loading…
Reference in New Issue
No description provided.
Delete Branch "feature/task-cover"
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?
This PR implements setting an uploaded attachment as cover image for a task which will make it show up on the kanban board. Implementing this with attachments allowed to re-use a bunch of existing logic around upload, permissions and retrieval of the actual image file.
Depends on #2459
Frontend PR for vikunja/api#1263
Resolves https://github.com/go-vikunja/frontend/issues/53
Related forum discussion: https://community.vikunja.io/t/feature-request-use-image-attachments-as-cover-image-on-kanban-cards-like-trello/351/2
Hi konrad!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://2460-feature-task-cover--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: feat(task): cover image for tasksto feat(task): cover image for tasks@ -81,0 +83,4 @@
class="attachment-info-meta-button"
@click.prevent.stop="setCoverImage(task.coverImageAttachmentId === a.id ? null : a)"
>
{{ task.coverImageAttachmentId === a.id ? $t('task.attachment.unsetAsCover') : $t('task.attachment.setAsCover') }}
Picky:
line wrap
Done.
@ -170,0 +166,4 @@
import {useTaskStore} from '@/stores/tasks'
import {useI18n} from 'vue-i18n'
const taskStore = useTaskStore()
Because I never mentioned this I'm noting this down here.
I currently am following this order roughly when creating vue files:
- then loose groups of (depending on what makes sense)
When I migrated to script setup I usually tried to at least follow this until init state.
I'm happy to use a different order or no order at all, just wanted to inform about my though process here :)
In case we want to follow this in the future: Should I mention this somewhere in the docs?
@ -170,0 +167,4 @@
import {useI18n} from 'vue-i18n'
const taskStore = useTaskStore()
const {t} = useI18n()
add global scope
Done.
@ -170,2 +177,4 @@
editEnabled: true,
})
// FIXME: this should go through the store
Maybe now that we have a two-way binding we should use
v-model
?I didn't do that because I feel like a
v-model
for the attachment component would be the attachments itself. But we need to pass data to the parent task, not the attachments.I'd like to refactor the task detail view so that the task in there is loaded in through the store and all updates happen through the store. That way we would not need a
v-model
here and solve things like this and this.Let's plan a bit when we do this, but sounds good :)
Just that I understand correctly:
So in this case you would want to pass in the taskId again in the future.
And to manage the attachments we would call store methods from inside the component?
Or maybe pass the whole task object into the component, but don't bubble it back up but instead update it through the store.
@ -172,1 +180,4 @@
// FIXME: this should go through the store
const emit = defineEmits(['task-changed'])
const attachmentService = shallowReactive(new AttachmentService())
The reason I do use
shallowReactive
for services is: we can easily watch the loading state.I'm not even sure if we have a deep nested state in any service so maybe reactive would actually be identical.
My thought was here only that in order to have
loading
reactiveshallowReactive
is enough.@ -239,0 +252,4 @@
copy(generateAttachmentUrl(props.task.id, attachment.id))
}
async function setCoverImage(attachment: IAttachment | null) {
Put this function as named exported function in the task store.
This way we can in theory reuse this whereever we want.
Having all actions like these in the store might help us in the future to easier implement such functionality with quick actions.
Obviously the function in the store then needs to accept the task additionally to the attachment as paramenter.
Done.
@ -30,0 +17,4 @@
alt=""
class="cover-image"
/>
<div class="p-2">
Why the extra wrapper instead of adding
p-2
to the task-id?Because it's not only the task id but every other element that's coming after that which would require the spacing. Adding a wrapper seemed simpler.
Before I added the cover image element the padding was on the card element itself.
Oversaw the other elements. Makes sense!
@ -142,0 +121,4 @@
}
}
function openTaskDetail() {
We should use here a router-link instead. This way we also support right click => 'open in new tab' and middle click.
I tried that, but it has weired side effects like styling (easily fixable) and not realiably opening the task detail view on click (not sure why, probably related to the drag and drop stuff). Let's leave this for another PR, I feel like this is kind of out of scope for this one.
Makes sense!
@ -5,6 +5,8 @@ import type { IUser } from '@/modelTypes/IUser'
import type { IFile } from '@/modelTypes/IFile'
import type { IAttachment } from '@/modelTypes/IAttachment'
export const SUPPORTED_IMAGE_SUFFIX = ['.jpg', '.png', '.bmp', '.gif']
Align this with
SUPPORTED_SUFFIX
inattachments.vue
.But that does not exist in the vue component anymore?
🤣
I searched locally and didn't think of that you obviously moved this from one file to another…
@ -219,3 +219,3 @@
<attachments
:edit-enabled="canWrite"
:task-id="taskId"
:task="task"
Use v-model for task
See my comment above
a491d418ac
to0ca6588139
@ -78,6 +78,17 @@
>
{{ $t('misc.delete') }}
</BaseButton>
<BaseButton
I feel like only changing the wording of the button isn't enough indication for the user which cover image is currently set, if he has the task detail open. When you change it you get a notification which is fine. But if you have a list of attachments you would need to scroll through them and check the wording of the buttons in order to see the current state.
I'm not sure what a good solution could be here. Maybe something like the icon used in the GitHub tread: https://github.com/go-vikunja/frontend/issues/53#issuecomment-1139502638.
I think only the current image should have the icon though, in order to have less visual pollution.
If all attachments would have the icon it would be hard to scan again.
Open for alternatives
Maybe even add a little badge (like the "done" we have for done tasks on the kanban board) next to the attachment that's used as cover?
I like that. Could also be like the pills used for selected users in the multiselect. this way it could also have an 'x' to remove it again.
I've now added this:
Looks good! :)
@ -695,2 +695,3 @@
"copyUrl": "Copy URL",
"copyUrlTooltip": "Copy the url of this attachment for usage in text"
"copyUrlTooltip": "Copy the url of this attachment for usage in text",
"setAsCover": "Set as cover image",
I like the short version of https://github.com/go-vikunja/frontend/issues/53#issuecomment-1139502638
"Make cover" – "Remove cover"
'Unset' sounds so bulky
Done.
1038d7b764
toa7731370a0