Show error icon / message when thumbnail can't be generated in-app #16843

jay-p-b-7span · 2022-12-17T12:24:49Z

… be generated (e.g. image file too big)

Description

Fixes #16440, fixes ENG-139

Type of Change

Requirements Checklist

New / updated tests are included
All tests are passing locally
Performed a self-review of the submitted code

If adding a new feature:

Documentation was added/updated. PR:

… be generated (e.g. image file too big)

…issue-16440

paescuj

Thanks @jay-p-b-7span for your pull request! ❤️
Would you be able to provide a quick screenshot to get an idea of how it's going to look?

app/src/components/v-image.vue

jay-p-b-7span · 2022-12-20T05:49:17Z

I have attached screenshots.

freekrai · 2023-01-16T21:43:40Z

So I get the

[ILLEGAL_ASSET_TRANSFORMATION] Image is too large to be transformed, or image size couldn't be determined.

error but it still creates the record in the database just doesn't actually upload the file, is this the behaviour you'd expect to see?

paescuj · 2023-01-16T22:40:44Z

@freekrai It should get uploaded in any case, but not displayed as the generation of a thumbnail might take up too many resources for larger images. The idea of this pull request is that the user is informed accordingly in such a case, as otherwise it is unclear why no image is displayed.

As for this pull request, I'm currently wondering the following:

Would there possibly even be a more suitable icon, so that you can see directly what the display issue is?
I'm not yet completely satisfied with the design in the detailed view. Could we maybe show a thumbnail with the error icon there as well? This way it would be clear that the image is usually displayed at that place. And maybe we could embed the error message somehow?

freekrai · 2023-01-16T22:47:29Z

@paescuj yeah I do see the file in uploads so that works as expected and yes, it can be confusing as when you first upload you don't see why there is no thumbnail showing unless you click on the image itself and go into the db record.

paescuj · 2023-01-16T22:51:30Z

@paescuj yeah I do see the file in uploads so that works as expected and yes, it can be confusing as when you first upload you don't see why there is no thumbnail showing unless you click on the image itself and go into the db record.

I completely agree with you!

freekrai · 2023-01-18T20:52:16Z

That said, this does do what's expected so it can be approved and further tweaks made in a future PR.

freekrai

👍🏻

app/src/components/v-image.vue

rijkvanzanten · 2023-01-18T21:24:35Z

app/src/views/private/components/file-preview.vue

+async function onErrorHandle() {
+	if (!props.src) return;
+	try {
+		await api.get(props.src);
+	} catch (err: any) {
+		imgError.value = true;
+		imageError.value = err;
+	}
+}


This error handling is fetching the image fresh to extract the error, even though the v-image that throws the error already emits the error. Lets reuse the error emitted by v-image

I completely agree with you @rijkvanzanten to reuse code. But error emitted by v-image has responseType ArrayBuffer which v-error component would not be able to handle it. Therefore result might look like attached in screenshot.

Reason of making new request

In order to address this responseType ArrayBuffer, I had made new request.

Solution

1st option
We can handle converting Responsetype to String in v-image Component.

2nd option
We can handle this conversion of responseType in v-error Component.

Let me know for any updates.

app/src/views/private/components/file-preview.vue

rijkvanzanten · 2023-01-18T21:25:32Z

That said, this does do what's expected so it can be approved and further tweaks made in a future PR.

It might act like expected, but the code isn't quite there yet 🙂

paescuj · 2023-01-18T21:31:37Z

That said, this does do what's expected so it can be approved and further tweaks made in a future PR.

It might act like expected, but the code isn't quite there yet 🙂

I agree! Besides, I'd like to have at least this point discussed / solved before having it merged (#16843 (comment)):

I'm not yet completely satisfied with the design in the detailed view. Could we maybe show a thumbnail with the error icon there as well? This way it would be clear that the image is usually displayed at that place. [...]

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

github-actions · 2023-04-11T20:02:09Z

Hi @freekrai, @jay-p-b-7span!

Thank you for contributing to Directus! Before we consider your Pull Request, we ask that you sign our Contributor License Agreement (CLA). This is only required for your first Pull Request.

Please review the CLA, and sign it by adding your GitHub username to the contributors.yml file. Thanks!

rijkvanzanten · 2023-04-11T20:05:41Z

I'll close this for now while we're researching a solution for #16843 (comment)

This Commit solves issue 16440 No visual feedback if thumbnail cannot…

334a384

… be generated (e.g. image file too big)

jay-p-b-7span mentioned this pull request Dec 17, 2022

No visual feedback if thumbnail cannot be generated (e.g. image file too big) #16440

Closed

rijkvanzanten changed the title ~~This PR solves issue 16440 No visual feedback if thumbnail cannot…~~ Show error icon / message when thumbnail can't be generated in-app Dec 18, 2022

jay-p-b-7span added 3 commits December 19, 2022 20:26

Removed comments

d16575f

Merge branch 'main' of https://github.com/directus/directus into fix/…

ec032e2

…issue-16440

Optimized performance by removing png and used material icon

53c5460

paescuj requested changes Dec 20, 2022

View reviewed changes

app/src/components/v-image.vue Outdated Show resolved Hide resolved

app/src/components/v-image.vue Outdated Show resolved Hide resolved

Requested changes are ready to be reviewed

30cc363

br41nslug and others added 2 commits December 30, 2022 17:23

Merge branch 'main' into fix/issue-16440

7a36b56

Merge branch 'main' into fix/issue-16440

f650ae8

Merge branch 'main' into fix/issue-16440

8dcde91

freekrai approved these changes Jan 18, 2023

View reviewed changes

rijkvanzanten requested changes Jan 18, 2023

View reviewed changes

freekrai and others added 2 commits January 18, 2023 15:40

Update app/src/views/private/components/file-preview.vue

757ef6d

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

Linter warnings resolved

24434d0

jay-p-b-7span requested review from rijkvanzanten and freekrai and removed request for rijkvanzanten and freekrai January 23, 2023 08:57

rijkvanzanten removed their request for review February 15, 2023 22:16

Merge branch 'main' into fix/issue-16440

2f594ae

rijkvanzanten closed this Apr 11, 2023

github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show error icon / message when thumbnail can't be generated in-app #16843

Show error icon / message when thumbnail can't be generated in-app #16843

jay-p-b-7span commented Dec 17, 2022 •

edited by rijkvanzanten

paescuj left a comment

jay-p-b-7span commented Dec 20, 2022

freekrai commented Jan 16, 2023

paescuj commented Jan 16, 2023

freekrai commented Jan 16, 2023

paescuj commented Jan 16, 2023

freekrai commented Jan 18, 2023

freekrai left a comment

rijkvanzanten Jan 18, 2023

jay-p-b-7span Jan 19, 2023

jay-p-b-7span Jan 31, 2023

rijkvanzanten commented Jan 18, 2023

paescuj commented Jan 18, 2023 •

edited

github-actions bot commented Apr 11, 2023

rijkvanzanten commented Apr 11, 2023

Show error icon / message when thumbnail can't be generated in-app #16843

Show error icon / message when thumbnail can't be generated in-app #16843

Conversation

jay-p-b-7span commented Dec 17, 2022 • edited by rijkvanzanten

Description

Type of Change

Requirements Checklist

paescuj left a comment

Choose a reason for hiding this comment

jay-p-b-7span commented Dec 20, 2022

freekrai commented Jan 16, 2023

paescuj commented Jan 16, 2023

freekrai commented Jan 16, 2023

paescuj commented Jan 16, 2023

freekrai commented Jan 18, 2023

freekrai left a comment

Choose a reason for hiding this comment

rijkvanzanten Jan 18, 2023

Choose a reason for hiding this comment

jay-p-b-7span Jan 19, 2023

Choose a reason for hiding this comment

Reason of making new request

Solution

jay-p-b-7span Jan 31, 2023

Choose a reason for hiding this comment

rijkvanzanten commented Jan 18, 2023

paescuj commented Jan 18, 2023 • edited

github-actions bot commented Apr 11, 2023

rijkvanzanten commented Apr 11, 2023

jay-p-b-7span commented Dec 17, 2022 •

edited by rijkvanzanten

paescuj commented Jan 18, 2023 •

edited