fix(files): Add proper visual loading feedback for image preview

[susnux]

Summary

Currently only a white square is shown while loading the icon, this add a loading indicator.
(Especially the previous one had a short flash when both the background and icon are shown at the same time).

Screen recording

Before

before.mp4

After

after.mp4

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[skjnldsv]

Currently only a white square is shown while loading the icon, this add a loading indicator.
(Especially the previous one had a short flash when both the background and icon are shown at the same time).

This is supposed to be a placeholder.
Somehow it's disabled

server/apps/files/src/components/FilesListVirtual.vue

Line 498 in e210caf

// animation: preview-gradient-fade 1.2s ease-in-out infinite;

Imho, loading spinners are good for individual element loading.
For a lot of them (like here, in a list), it's distracting and a loading placeholder is what should be used (we do it in many other places)

[skjnldsv]

How t was supposed to look like
I think I forgot to re-enable it after 014a57e#diff-13362ff2e0fefd60f2365d996fe145183e796ed346aeb8c28421a44c720dfeba

[marcoambrosini]

Out of the two, I think that the loading spinner looks nicer and is just as clear

[susnux]

Makes sense, like a loading skeleton.

So I am fine with whatever the @nextcloud/designers prefer :)

[skjnldsv]

Out of the two, I think that the loading spinner looks nicer and is just as clear

For the record, my point is not about if it's clearer or not, but the fact that 20 spinners shown drag too much attention for very little value (it's just a preview) :)

Anyway, as Ferdinand said: I am fine with whatever the @nextcloud/designers prefer 😉

[marcoambrosini]

The gif you posted @skjnldsv seem to draw more attention than the little spinner in @susnux video. But I'm fine either way tbh :)

[nimishavijay]

For a lot of them (like here, in a list), it's distracting and a loading placeholder is what should be used (we do it in many other places)

I would agree here, the square being distracting is rather a result of the color of the skeleton screen component being too light in dark mode IMO, so darkening the color of the squares would already make it look less busy. What do you think?

[skjnldsv]

darkening the color of the squares would already make it look less busy. What do you think?

Seconded 👍

[jancborchardt]

Looks nice with the skeleton squares, and agree with the feedback of adjusting the color a bit (lightening/darkening).

An additional thing: When it’s a file which does not have a preview, like a PDF for example – why don’t we just instantly show the PDF filetype icon?

[skjnldsv]

An additional thing: When it’s a file which does not have a preview, like a PDF for example – why don’t we just instantly show the PDF filetype icon?

Because we don't always know if there is a preview. It can be available but not generated.
There is a ticket somewhere to have a property saying if the preview for thie mime is enabled or not, that's a different approach

[jancborchardt]

Because we don't always know if there is a preview. It can be available but not generated.

Right, so what I’m saying is that we can show the filetype icon instantly. Preview could be shown as soon as available. That makes things feel more instant instead of having loading bars/skeletons here.