mdx-editor / editor

A rich text editor React component for markdown
https://mdxeditor.dev
MIT License
1.75k stars 139 forks source link

Added disableImageUpload to imagePlugin #537

Closed rissois closed 1 month ago

rissois commented 1 month ago

Configuration flag for imagePlugin to disable image upload. Ideal for projects that are not self-hosting images.

NOTE: One further improvement to this PR would be to eliminate the dropEffect when an outside image is brought onto the page.

petyosi commented 1 month ago

This makes sense, I will review and merge in a few days

rissois commented 1 month ago

@petyosi

Sorry, I rolled back on the onDrop changes. First, just to confirm the different interactions:

  1. Image dialog upload > call imageUploadHandler > link to external URL
  2. Image dialog URL > link to external URL
  3. Copy and paste
    • from within editor > link to external URL
    • from other website > link to external URL
    • from computer file > call imageUploadHandler > link to external URL
  4. Drag and drop
    • from within editor > link to external URL
    • from other website > link to external URL
    • from computer file > call imageUploadHandler > link to external URL

I think the easiest solution is to check the src for /^https?:\/\// prior to imageeUploadHandler in order to block all local, non-HTTP(S) images. Also want to remove the dropEffect/effectAllowed. Unfortunately I did not know how to make those changes in the right location to block both Copy and paste + Drag and drop. Another major issue I was running into in development is that drag-and-drop in the Ladle example produces an element instead of a markdown ![](). Therefore, I'm not really sure I am getting the true behavior.

Sorry I was not able to complete the PR on my own.

petyosi commented 1 month ago

@rissois Thank you - your idea is right, but I don't think we need an additional parameter. Not having an upload handler should be enough to disable the upload button and the D & D from the drag start.

I've published a fix for this in v3.9.1. Let me know if this resolves the problem for you.

rissois commented 1 month ago

@petyosi Thanks! That makes a lot of sense, I just wasn't sure the implications of them being mutually exclusive.

Unfortunately, the fix seems to have created a few bugs:

  1. Can no longer use ImageDialog to add a link. Possible solution: add a hidden input field to still register the file (if that is the true root of the problem)
    {imageUploadHandler === null ? (
    <input type="hidden" accept="image/*" {...register('file')} />
    ) : (
    <div className={styles.formField}>
    <label htmlFor="file">{t('uploadImage.uploadInstructions', 'Upload an image from your device:')}</label>
    <input type="file" accept="image/*" {...register('file')} />
    </div>
    )}
  2. Copy and paste from another website creates <img height="0" width="0" title="filename.jpg" src="https://...." /> instead of !()[https://...]. The lack of dimensions effective hides the image (this one stumped me)
  3. Unable to drag and drop !()[https://...] images. Possible solution: check if src is originating from outside of a website
    if (!imageUploadHandler) {
    const isExternalImage = /^https?:\/\//.test(node.__src)
    if (isExternalImage) {
      return false
    }
    }
  4. ImageWithNoBackend example was still given imageUploadHandler. Just need to delete that one line.
petyosi commented 1 month ago

@rissois thank you - some of those things should be a matter of configuration that's not related to the upload handler - to be more specific, some sort of a validation of which images can be accepted.

I will review what you've described at some point, but things are fairly busy on my side, so if you have the capacity, I'm happy to accept a PR on the matters.

rissois commented 1 month ago

@petyosi Happy to help, I managed to figure out how to separate the image types in a method more consistent with your fix. Please see PR #549