Skip to content

Conversation

@petschki
Copy link
Member

@petschki petschki commented Oct 14, 2025

Closes #1491
Successor of #1492

This PR implements:

  1. A new pattern option upload-add-immediately which defaults to true. This fixes usability issues in the TinyMCE link/image modal.
  2. Button label changes to "Select or Upload" when upload is activated
  3. Do not close upload column when uploading a file fails
  4. minor tinymce pattern cleanup because of removed "upload" tab in modal

@petschki petschki requested review from erral and frapell October 14, 2025 12:50
@erral
Copy link
Member

erral commented Oct 14, 2025

I tested the inmediately-add-and-select feature and works as expected.

I tested also the upload fails, and the error is shown in the dialog, so I would say that is expected too.

@erral
Copy link
Member

erral commented Oct 14, 2025

The only thing I found is that the Image browser lets you uploading an arbitrary PDF file which then is inserted in TinyMCE which it shouldn't be possible.

@petschki
Copy link
Member Author

petschki commented Oct 14, 2025

The only thing I found is that the Image browser lets uploading an arbitrary PDF file which then is inserted in TinyMCE which it shouldn't be possible.

Yeah, saw that too. Dropzone.js has an option acceptedFiles which we can narrow down to image/* mimetype for only accepting image types. But ideally this is configurable on our side too ... maybe uploadAcceptedMimeTypes or something like that. Then the tinymce pattern can set this when firing up the ImageModal ... since I have all files open, I'll check that now 😉

@petschki
Copy link
Member Author

@erral mimetype restriction argument is implemented and image modal now only allows "image/*" mimetype. The only thing I encountered is when dropping a disallowed mimetype to the dropzone ... this should be more obvious that the dropped file is not allowed... selecting with "browse" is OK, because the file brower is restricted to the mimetype.

@erral
Copy link
Member

erral commented Oct 14, 2025

@erral mimetype restriction argument is implemented and image modal now only allows "image/*" mimetype. The only thing I encountered is when dropping a disallowed mimetype to the dropzone ... this should be more obvious that the dropped file is not allowed... selecting with "browse" is OK, because the file brower is restricted to the mimetype.

Yes I have seen that too :)

But I think it is working.

…. pat-tinymce uses this to ensure only 'image/*' mimetypes in image modal.
@petschki petschki force-pushed the petschki-immediate-add-uploads branch from e868cef to f9d017a Compare October 14, 2025 15:13
@erral
Copy link
Member

erral commented Oct 16, 2025

@petschki do you want me to further test this? I see some changes after my previous tests...

@petschki
Copy link
Member Author

Currently I'm waiting for @frapell feedback but I think you have tested the uploadAcceptedMimetypes parameter already, did you? That was the last change ...

@erral
Copy link
Member

erral commented Oct 16, 2025

Currently I'm waiting for @frapell feedback but I think you have tested the uploadAcceptedMimetypes parameter already, did you? That was the last change ...

Mmmm, I tried passing a custom mimetype in that option (adding {"uploadAcceptedMimetypes": ["image/jpg"]} in patternoptions, right?) but I can't make it work.

Perhaps I am missing something, I will keep testing it.

@petschki
Copy link
Member Author

petschki commented Oct 16, 2025

This has to be a string (like "image/*,application/pdf") ... see dropzone options for details: https://github.com/dropzone/dropzone/blob/main/src/options.js#L203

Basically like here https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/file#accept

@erral
Copy link
Member

erral commented Oct 16, 2025

This has to be a string (like "image/*,application/pdf") ... see dropzone options for details: https://github.com/dropzone/dropzone/blob/main/src/options.js#L203

Basically like here https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/file#accept

Ah, of course :)

It works as expected, but the behavior is a bit strange if I use the options like "doc" explained in the MDN docs, but setting full mimetypes works as expected.

Like previously, I can cheat my browser and select a non-accepted mimetype file, and it tries to upload but it finally doesnt' and doesn't report the error.

I could live with that, but if possible it would be nice to report an error there.

@frapell
Copy link
Member

frapell commented Oct 17, 2025

@petschki As far as I can test, this seems to be working perfectly, and I see the error printed if the @@fileUpload view fails.

I would focus in errors reported by normal usage by editors, and while it would be nice to show error messages in cases like the ones mentioned by @erral I think it is fine if someone manages to "cheat the browser" to not actually get a helpful error (In fact if they are messing with the devtools, they might know already what the error is about ;) )

Copy link
Member

@frapell frapell left a comment

Choose a reason for hiding this comment

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

LGTM !

@petschki petschki merged commit 5757d0b into master Oct 17, 2025
3 checks passed
@petschki petschki deleted the petschki-immediate-add-uploads branch October 17, 2025 09:41
@erral
Copy link
Member

erral commented Oct 17, 2025

I am merging the locales PR too.

@petschki
Copy link
Member Author

This (and others) is now released in plone.staticresources = 2.3.4 ... see https://github.com/plone/mockup/releases/tag/5.4.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pat-contentbrowser: When an upload fails, the pat-upload panel closes without indication of a failure

4 participants