Frontend: Proof of concept for image preview #2266
|
|
@ -27,89 +27,91 @@
|
|||
v-if="attachments.length > 0"
|
||||
class="files"
|
||||
>
|
||||
<!-- FIXME: don't use a for element that wraps other links / buttons
|
||||
Instead: overlay element with button that is inside.
|
||||
-->
|
||||
<a
|
||||
v-for="a in attachments"
|
||||
:key="a.id"
|
||||
class="attachment"
|
||||
@click="viewOrDownload(a)"
|
||||
>
|
||||
<div class="filename">
|
||||
{{ a.file.name }}
|
||||
<span
|
||||
v-if="task.coverImageAttachmentId === a.id"
|
||||
class="is-task-cover"
|
||||
>
|
||||
{{ $t('task.attachment.usedAsCover') }}
|
||||
</span>
|
||||
</div>
|
||||
<div class="info">
|
||||
<p class="attachment-info-meta">
|
||||
<i18n-t
|
||||
keypath="task.attachment.createdBy"
|
||||
scope="global"
|
||||
>
|
||||
<span v-tooltip="formatDateLong(a.created)">
|
||||
{{ formatDateSince(a.created) }}
|
||||
<table class="table table-striped">
|
||||
|
|
||||
<tr
|
||||
v-for="a in attachments"
|
||||
:key="a.id"
|
||||
konrad
commented
`grid-item` sounds like a child of a grid. Why not call it `attachment` as it was before?
|
||||
class="clickable"
|
||||
@click="viewOrDownload(a)"
|
||||
>
|
||||
<td class="preview-column">
|
||||
<FilePreview
|
||||
v-if="canPreview(a)"
|
||||
class="attachment-preview"
|
||||
:model-value="a"
|
||||
/>
|
||||
</td>
|
||||
<td>
|
||||
<div class="filename">
|
||||
{{ a.file.name }}
|
||||
<span
|
||||
v-if="task.coverImageAttachmentId === a.id"
|
||||
class="is-task-cover"
|
||||
>
|
||||
{{ $t('task.attachment.usedAsCover') }}
|
||||
</span>
|
||||
<User
|
||||
:avatar-size="24"
|
||||
:user="a.createdBy"
|
||||
:is-inline="true"
|
||||
/>
|
||||
</i18n-t>
|
||||
<span>
|
||||
{{ getHumanSize(a.file.size) }}
|
||||
</span>
|
||||
<span v-if="a.file.mime">
|
||||
{{ a.file.mime }}
|
||||
</span>
|
||||
</p>
|
||||
<img
|
||||
v-if="canPreview(a)"
|
||||
:src="blobUrls.get(a.id)"
|
||||
style="height: 20vh; min-height: 100px"
|
||||
alt=""
|
||||
>
|
||||
<p>
|
||||
<BaseButton
|
||||
v-tooltip="$t('task.attachment.downloadTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.prevent.stop="downloadAttachment(a)"
|
||||
>
|
||||
{{ $t('misc.download') }}
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-tooltip="$t('task.attachment.copyUrlTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.stop="copyUrl(a)"
|
||||
>
|
||||
{{ $t('task.attachment.copyUrl') }}
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="editEnabled"
|
||||
v-tooltip="$t('task.attachment.deleteTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.prevent.stop="setAttachmentToDelete(a)"
|
||||
>
|
||||
{{ $t('misc.delete') }}
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="editEnabled"
|
||||
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')
|
||||
}}
|
||||
</BaseButton>
|
||||
</p>
|
||||
</div>
|
||||
</a>
|
||||
</div>
|
||||
<div class="info">
|
||||
<p class="attachment-info-meta">
|
||||
<i18n-t
|
||||
keypath="task.attachment.createdBy"
|
||||
scope="global"
|
||||
>
|
||||
<span v-tooltip="formatDateLong(a.created)">
|
||||
{{ formatDateSince(a.created) }}
|
||||
</span>
|
||||
<User
|
||||
:avatar-size="24"
|
||||
:user="a.createdBy"
|
||||
:is-inline="true"
|
||||
/>
|
||||
</i18n-t>
|
||||
<span>
|
||||
{{ getHumanSize(a.file.size) }}
|
||||
|
konrad
commented
I would extract this out into its own component, which takes the attachment, fetches the image and shows it next to the attachment. We could extend this further to show some kind of icon for pdfs or other file types later on. I would extract this out into its own component, which takes the attachment, fetches the image and shows it next to the attachment. We could extend this further to show some kind of icon for pdfs or other file types later on.
Elscrux
commented
Good point, I did that Good point, I did that
|
||||
</span>
|
||||
<span v-if="a.file.mime">
|
||||
{{ a.file.mime }}
|
||||
</span>
|
||||
</p>
|
||||
<p>
|
||||
<BaseButton
|
||||
v-tooltip="$t('task.attachment.downloadTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.prevent.stop="downloadAttachment(a)"
|
||||
>
|
||||
{{ $t('misc.download') }}
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-tooltip="$t('task.attachment.copyUrlTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.stop="copyUrl(a)"
|
||||
>
|
||||
{{ $t('task.attachment.copyUrl') }}
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="editEnabled"
|
||||
v-tooltip="$t('task.attachment.deleteTooltip')"
|
||||
class="attachment-info-meta-button"
|
||||
@click.prevent.stop="setAttachmentToDelete(a)"
|
||||
>
|
||||
{{ $t('misc.delete') }}
|
||||
|
konrad
commented
This needs a tooltip This needs a tooltip
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="editEnabled"
|
||||
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')
|
||||
}}
|
||||
</BaseButton>
|
||||
</p>
|
||||
</div>
|
||||
</td>
|
||||
</tr>
|
||||
</table>
|
||||
</div>
|
||||
|
||||
<x-button
|
||||
|
|
@ -174,7 +176,7 @@
|
|||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import {ref, shallowReactive, computed, onMounted} from 'vue'
|
||||
import {ref, shallowReactive, computed} from 'vue'
|
||||
import {useDropZone} from '@vueuse/core'
|
||||
|
||||
import User from '@/components/misc/user.vue'
|
||||
|
|
@ -194,6 +196,7 @@ import {useCopyToClipboard} from '@/composables/useCopyToClipboard'
|
|||
import {error, success} from '@/message'
|
||||
import {useTaskStore} from '@/stores/tasks'
|
||||
import {useI18n} from 'vue-i18n'
|
||||
import FilePreview from '@/components/tasks/partials/file-preview.vue'
|
||||
|
||||
const {
|
||||
task,
|
||||
|
|
@ -228,20 +231,6 @@ function downloadAttachment(attachment: IAttachment) {
|
|||
|
||||
const filesRef = ref<HTMLInputElement | null>(null)
|
||||
|
||||
const blobUrls = ref<Map<number, string>>(new Map())
|
||||
|
||||
onMounted(() => {
|
||||
fetchBlobUrls(attachments.value as IAttachment[])
|
||||
})
|
||||
|
||||
async function fetchBlobUrls(attachments: IAttachment[]) {
|
||||
for (const attachment of attachments) {
|
||||
const blobUrl = await attachmentService.getBlobUrl(attachment)
|
||||
if (canPreview(attachment)) {
|
||||
blobUrls.value.set(attachment.id, blobUrl)
|
||||
}
|
||||
}
|
||||
}
|
||||
function uploadNewAttachment() {
|
||||
const files = filesRef.value?.files
|
||||
|
||||
|
|
@ -458,6 +447,18 @@ async function setCoverImage(attachment: IAttachment | null) {
|
|||
}
|
||||
}
|
||||
|
||||
.preview-column {
|
||||
|
konrad marked this conversation as resolved
Outdated
konrad
commented
Please use Please use `rem` or an existing scss variable (same for the other occurences)
Elscrux
commented
Done Done
|
||||
max-width: 75px;
|
||||
}
|
||||
|
||||
.attachment-preview {
|
||||
max-height: 75px;
|
||||
}
|
||||
|
konrad
commented
What happens when the item is clicked? What happens when the item is clicked?
Elscrux
commented
The image preview is shown, like previously. Nothing changed in that regard. Previously, an tag was used to have the cursor change to a clickable cursor. This comment was there and I fixed it by adding this clickable class.
The image preview is shown, like previously. Nothing changed in that regard. Previously, an <a> tag was used to have the cursor change to a clickable cursor. This comment was there and I fixed it by adding this clickable class.
> FIXME: don't use a for element that wraps other links / buttons
konrad
commented
In terms of a11y, if it's actually clickable, it should be a button or a. Divs are usually not clickable. In terms of a11y, if it's actually clickable, it should be a button or a. Divs are usually not clickable.
Elscrux
commented
Fair. I changed this and it has a little different feel to it now. Let me know what you think! Fair. I changed this and it has a little different feel to it now. Let me know what you think!
konrad
commented
Looks good! Will do a proper review when I'm back at my computer, early next week. Looks good! Will do a proper review when I'm back at my computer, early next week.
|
||||
|
||||
.clickable {
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.is-task-cover {
|
||||
background: var(--primary);
|
||||
color: var(--white);
|
||||
|
|
|
|||
34
frontend/src/components/tasks/partials/file-preview.vue
Normal file
34
frontend/src/components/tasks/partials/file-preview.vue
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
<template>
|
||||
<img
|
||||
|
konrad
commented
Can you make this always the same size? Use Can you make this always the same size? Use [`object-fit: cover;`](https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit) to let the image fill the container nicely.
Elscrux
commented
I included it, but it didn't seem like it made a difference I included it, but it didn't seem like it made a difference
konrad
commented
Did you set a fixed with and height for the image? Did you set a fixed with and height for the image?
Elscrux
commented
Yes, it's at 4rem now Yes, it's at 4rem now
|
||||
:src="blobUrl"
|
||||
alt="Attachment preview"
|
||||
>
|
||||
|
konrad
commented
Please do not use inline styles. This image should have a rectangular fixed size, then the Please do not use inline styles.
This image should have a rectangular fixed size, then the `object-fit` property will make sure it looks good.
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import {type PropType, ref, shallowReactive, watchEffect} from 'vue'
|
||||
import AttachmentService from '@/services/attachment'
|
||||
|
konrad
commented
Can you use a simple file icon here? An image icon suggests the file is an image, which it is not. Can you use a simple [file icon](https://fontawesome.com/icons/file?f=classic&s=regular) here? An image icon suggests the file is an image, which it is not.
konrad
commented
Also please make the file icon in a dark gray, similar to the attachment title. Also please make the file icon in a dark gray, similar to the attachment title.
|
||||
import type { IAttachment } from '@/modelTypes/IAttachment'
|
||||
|
||||
const props = defineProps({
|
||||
modelValue: {
|
||||
type: Object as PropType<IAttachment>,
|
||||
default: undefined,
|
||||
},
|
||||
})
|
||||
|
||||
const attachmentService = shallowReactive(new AttachmentService())
|
||||
const blobUrl = ref<string | undefined>(undefined)
|
||||
|
||||
watchEffect(async () => {
|
||||
if (props.modelValue) {
|
||||
blobUrl.value = await attachmentService.getBlobUrl(props.modelValue)
|
||||
}
|
||||
})
|
||||
</script>
|
||||
|
||||
<style scoped lang="scss">
|
||||
img {
|
||||
border-radius: 0.5rem;
|
||||
|
konrad marked this conversation as resolved
Outdated
konrad
commented
Please use Please use `$radius` here so that the radius is consistent with other elements.
Elscrux
commented
Done! Done!
|
||||
}
|
||||
|
konrad
commented
Adding a Adding a `width: 100%` here will make it look like what I meant (will share a screenshot later)
|
||||
</style>
|
||||
Loading…
Reference in New Issue
Block a user
Please do not use a table for multi-column layout.
I used a grid instead now