Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions lib/Controller/AttachmentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,18 @@ public function getImageFile(string $imageFileName, string $shareToken = '',
$userId = $this->getUserId();
$imageFile = $this->attachmentService->getImageFile($documentId, $imageFileName, $userId, $preferRawImage === 1);
}
return $imageFile !== null
? new DataDownloadResponse(

if ($imageFile !== null) {
$response = new DataDownloadResponse(
$imageFile->getContent(),
$imageFile->getName(),
$this->getSecureMimeType($imageFile->getMimeType())
)
: new DataResponse('', Http::STATUS_NOT_FOUND);
);
$response->cacheFor(3600 * 24, false, true);
return $response;
}

return new DataResponse('', Http::STATUS_NOT_FOUND);
} catch (Exception $e) {
$this->logger->error('getImageFile error', ['exception' => $e]);
return new DataResponse('', Http::STATUS_NOT_FOUND);
Expand Down
23 changes: 19 additions & 4 deletions lib/Service/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\FilesMetadata\IFilesMetadataManager;
use OCP\IPreview;
use OCP\IURLGenerator;
use OCP\Lock\LockedException;
Expand All @@ -39,6 +40,7 @@ public function __construct(
private IMimeTypeDetector $mimeTypeDetector,
private IURLGenerator $urlGenerator,
private IFilenameValidator $filenameValidator,
private IFilesMetadataManager $filesMetadataManager,
) {
}

Expand Down Expand Up @@ -219,21 +221,34 @@ public function getAttachmentList(int $documentId, ?string $userId = null, ?Sess

$attachments = [];
$userFolder = $userId !== null ? $this->rootFolder->getUserFolder($userId) : null;

$fileNodes = [];
$fileIds = [];
foreach ($attachmentDir->getDirectoryListing() as $node) {
if (!($node instanceof File)) {
// Ignore anything but files
continue;
if ($node instanceof File) {
// we only want Files
$fileNodes[] = $node;
$fileIds[] = $node->getId();
}
}

// this is done outside the loop for efficiency
$metadataMap = $this->filesMetadataManager->getMetadataForFiles($fileIds);

foreach ($fileNodes as $node) {
$isImage = in_array($node->getMimetype(), AttachmentController::IMAGE_MIME_TYPES, true);
$name = $node->getName();
$fileId = $node->getId();
$metadata = $metadataMap[$fileId] ?? null;
$attachments[] = [
'fileId' => $node->getId(),
'fileId' => $fileId,
'name' => $name,
'size' => Util::humanFileSize($node->getSize()),
'mimetype' => $node->getMimeType(),
'mtime' => $node->getMTime(),
'isImage' => $isImage,
'davPath' => $userFolder?->getRelativePath($node->getPath()),
'metadata' => $metadata,
'fullUrl' => $isImage
? $this->urlGenerator->linkToRouteAbsolute('text.Attachment.getImageFile') . $urlParamsBase . '&imageFileName=' . rawurlencode($name) . '&preferRawImage=1'
: $this->urlGenerator->linkToRouteAbsolute('text.Attachment.getMediaFile') . $urlParamsBase . '&mediaFileName=' . rawurlencode($name),
Expand Down
83 changes: 82 additions & 1 deletion src/nodes/ImageView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<template>
<NodeViewWrapper :contenteditable="isEditable">
<figure
ref="wrapper"
class="image image-view"
data-component="image-view"
:data-attachment-type="attachmentType"
Expand Down Expand Up @@ -105,6 +106,12 @@
@close="showImageModal = false" />
</div>
</div>
<div v-else-if="canDisplayPlaceholder" class="image__placeholder">
<NcBlurHash
:hash="imageBlurhash"
:style="blurhashSize"
aria-hidden="true" />
</div>
<div v-else class="image-view__cant_display">
<transition name="fade">
<div v-show="loaded" class="image__caption">
Expand All @@ -128,7 +135,9 @@
<script>
import ClickOutside from 'vue-click-outside'
import NcButton from '@nextcloud/vue/components/NcButton'
import NcBlurHash from '@nextcloud/vue/components/NcBlurHash'
import { showError } from '@nextcloud/dialogs'
import { logger } from '../helpers/logger.js'
import ShowImageModal from '../components/ImageView/ShowImageModal.vue'
import { useAttachmentResolver } from '../components/Editor.provider.js'
import { emit } from '@nextcloud/event-bus'
Expand All @@ -149,6 +158,7 @@
ImageIcon,
DeleteIcon,
NcButton,
NcBlurHash,
ShowImageModal,
NodeViewWrapper,
},
Expand All @@ -160,7 +170,13 @@
data() {
return {
attachment: null,
attachmentPromise: null,

Check warning on line 173 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L173

Added line #L173 was not covered by tests
imageLoaded: false,
imageWidth: 0,
imageHeight: 0,
wrapperWidth: 0,
resizeObserver: null,
imageBlurhash: null,

Check warning on line 179 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L175-L179

Added lines #L175 - L179 were not covered by tests
loaded: false,
failed: false,
showIcons: false,
Expand Down Expand Up @@ -199,6 +215,25 @@

return this.loaded && this.imageLoaded
},
canDisplayPlaceholder() {
return this.imageHeight > 0
},
blurhashSize() {
if (this.imageWidth > 0 && this.imageHeight > 0) {
const ratio = this.imageWidth / this.imageHeight
const newWidth =
this.wrapperWidth - 12 > this.imageWidth
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed not a blocker but would be nice to base this and the paddings on the image view on the css variables so they adjust if designers change their minds about spacing.

? this.imageWidth
: this.wrapperWidth - 12
const newHeight = newWidth / ratio

Check warning on line 228 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L218-L228

Added lines #L218 - L228 were not covered by tests

return {
width: `${newWidth}px`,
height: `${newHeight}px`,
}
}
return {}
},

Check warning on line 236 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L230-L236

Added lines #L230 - L236 were not covered by tests
src: {
get() {
return this.node.attrs.src || ''
Expand Down Expand Up @@ -233,6 +268,10 @@
})
},
mounted() {
this.attachmentPromise = this.$attachmentResolver.resolve(this.src)
this.loadAttachmentMetadata()
this.setupResizeObserver()

Check warning on line 273 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L271-L273

Added lines #L271 - L273 were not covered by tests

this.$nextTick(() => {
// nextTick is necessary, intersection detection is slightly unreliable without it
const options = {
Expand All @@ -254,10 +293,43 @@
},
beforeUnmount() {
this.loadIntersectionObserver?.disconnect()
this.resizeObserver?.disconnect()

Check warning on line 296 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L296

Added line #L296 was not covered by tests
},
methods: {
setupResizeObserver() {
if (!this.$refs.wrapper) return

Check warning on line 300 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L299-L300

Added lines #L299 - L300 were not covered by tests

this.resizeObserver = new ResizeObserver((entries) => {
const width = entries[0].contentRect.width
if (width > 0) {
this.wrapperWidth = width
}
})

Check warning on line 307 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L302-L307

Added lines #L302 - L307 were not covered by tests

this.resizeObserver.observe(this.$refs.wrapper)
},
async loadAttachmentMetadata() {
try {
this.attachment = await this.attachmentPromise

Check warning on line 313 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L309-L313

Added lines #L309 - L313 were not covered by tests

const metadata = this.attachment?.metadata || null

Check warning on line 315 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L315

Added line #L315 was not covered by tests

if (metadata) {
const size = metadata['photos-size']?.value
this.imageWidth = size?.width || 0
this.imageHeight = size?.height || 0

Check warning on line 320 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L317-L320

Added lines #L317 - L320 were not covered by tests

this.imageBlurhash = metadata.blurhash?.value || null
}
} catch (err) {
// TODO: bump up to warn when the Photos dependency is gone (i.e., we can expect the metadata to exist)
logger.debug('Failed to load attachment metadata', { err })
}
},

Check warning on line 328 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L322-L328

Added lines #L322 - L328 were not covered by tests
async loadPreview() {
this.attachment = await this.$attachmentResolver.resolve(this.src)
if (!this.attachment) {
this.attachment = await this.attachmentPromise
}

Check warning on line 332 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L330-L332

Added lines #L330 - L332 were not covered by tests
if (!this.attachment.previewUrl) {
throw new Error('Attachment source was not resolved')
}
Expand All @@ -268,6 +340,9 @@
this.imageLoaded = true
this.loaded = true
this.attachmentSize = this.attachment.size
// once the image is loaded, we can stop tracking the container width
// since we only use it for sizing the placeholder
this.resizeObserver?.disconnect()

Check warning on line 345 in src/nodes/ImageView.vue

View check run for this annotation

Codecov / codecov/patch

src/nodes/ImageView.vue#L343-L345

Added lines #L343 - L345 were not covered by tests
}
img.onerror = (e) => {
reject(new LoadImageError(e, this.attachment.previewUrl))
Expand Down Expand Up @@ -426,6 +501,12 @@
height: 100px;
}

.image__placeholder {
padding: 7px 6px;
margin-bottom: 26px;
position: relative;
}

.image__main {
max-height: calc(100vh - 50px - 50px);
}
Expand Down
Loading