umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.41k stars 2.66k forks source link

Not possible to replace image on media item #9342

Closed bjarnef closed 3 years ago

bjarnef commented 3 years ago

In Umbraco 8.9.0 it doesn't seem to be possible to remove an existing file/image on a media item and replace it with a new one.

In "old days" you could remove the image, save the media item and upload a new one. However since "Upload image" property now be default is mandatory this trigger a validation error.

It used to be possible to remove the file from the media (without save) and select a new file and after this save the media item again.

bjarnef commented 3 years ago

This doesn't seem to work via drag-and-drop, but it does work if selecting file via the click event as a regular file upload. Has anything changes here regarding the drag-and-drop?

nul800sebastiaan commented 3 years ago

You can fix this by making the upload field not mandatory. This was changed for this issue: https://github.com/umbraco/Umbraco-CMS/issues/4302

What does this have to do with drag & drop?

nul800sebastiaan commented 3 years ago

I think the functionality is super awkward right now, from a quick chat with @nielslyngsoe we come to the following conclusion:

In fact, I think the "Delete file(s)" button is a bit antiquated, it should be a nice delete icon with a good confirmation message that explains that deleting the picked file will also delete it from disk. Similar to what you get on, for example, a block element:

image

image

nul800sebastiaan commented 3 years ago

I've tentatively marked this as breaking, as when the behavior changes, we always have some people who were relying on the old behavior and they will get surprised otherwise.

umbrabot commented 3 years ago

Hi @bjarnef,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

bjarnef commented 3 years ago

@nul800sebastiaan yes, but even with the Upload property as mandatory I recall it was possible to replace the image/file by removing the media, select the new media and then save.

This is still possible when using clicking the dropzone, which trigger the regular file upload, but not via drag and drop.

nul800sebastiaan commented 3 years ago

Okay, I can do that, what do I not understand correctly? And I don't understand what the dropzone has to do with it?

9342

bjarnef commented 3 years ago

@nul800sebastiaan just tested this on Umbraco v8.7.0

Replace image using click event (regular hidden file upload)

QlbaJzkz7x

Replace image via drag and drop to dropzone

75Ch7hjjhu

For some reason the second one part is not possible in v8.9.0

nul800sebastiaan commented 3 years ago

Ah yes, I see, the dropzone is also broken here, should ALSO be fixed! 👍

bjarnef commented 3 years ago

In v8.8 this PR https://github.com/umbraco/Umbraco-CMS/pull/8390 added some feedback to user on dragover, but it could probably bee enhance further. https://css-tricks.com/drag-and-drop-file-uploading/

I have submitted a new feature request for this here: https://github.com/umbraco/Umbraco-CMS/issues/9374

bjarnef commented 3 years ago

It seems this doesn't work as expected now https://github.com/umbraco/Umbraco-CMS/pull/8390/files#diff-ddd0f94516d96f3c9f7710bfb9741d50da035826ea5a8b34156008125366882dR24-R25

which seem to prevent from go into this function onFilesSelected: https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js#L277

when comment out the following it does seem to work:

e.preventDefault();
e.stopPropagation();
bjarnef commented 3 years ago

@nul800sebastiaan actually it seems this also prevent upload via drag and drop using File Upload datattype on e.g. document type. Upload via the click event does work though.

nul800sebastiaan commented 3 years ago

@bjarnef -- I didn't look into this issue very deeply but see if might be connected to https://github.com/umbraco/Umbraco-CMS/pull/9933 - do you think the fix for that solves this as well? I see the same code has been touched.

bjarnef commented 3 years ago

@nul800sebastiaan yes, PR https://github.com/umbraco/Umbraco-CMS/pull/9376 solves this issue. Is that what you meant? 😅

nul800sebastiaan commented 3 years ago

Yep! I wanted to know if I should close this issue and PR? :-)