Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Upload images functionnality #71

Open
wants to merge 23 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@roipoussiere
Copy link

roipoussiere commented Mar 4, 2019

Related to #46

This is a first attempt to an image upload functionality.

A video is better than 100 words, so here we start:

pr-upload-image

You can see that it supports multiples images input.

Client-side usage

We need to specify the server address where the images will be sent with the imageUploadEndpoint option:

var easyMde = new EasyMDE({
    element: document.getElementById('md-text-area'),
    hideIcons: ['image'],
    showIcons: ['upload-image'],
    imageUploadEndpoint: "https://example.com/uploadImage"
});

upload-image is a new icon dedicated to file upload, visually identical to the actual image button. So this will not change the behavior of current image icon, for backward compatibility.

Server-side usage

The server is supposed to save this image, and if it's successful, return a 200-OK HTTP response containing the relative path of the image.

To give you an example, this is the Python code I used at server-side:

@app.route('/imageUpload', methods=['POST'])
def upload_image():
    """
    Upload an image to the server and store it to the folder defined in
    IMAGE_UPLOAD_FOLDER in flask configuration.
    Note that the image folder must be created first and with write access.
    :return: The relative path of the uploaded file, or an error among with the
    corresponding HTTP error code.
    """
    from werkzeug.utils import secure_filename
    import os.path as op
    if 'image' not in request.files:
        return 'no_file_part', 400  # Bad request
    file = request.files['image']
    if not file.filename:
        return 'no_selected_file', 400  # Bad request
    if file and '.' in file.filename \
            and file.filename.rsplit('.', 1)[1].lower() in app.config['ALLOWED_IMAGES_TYPES']:
        file_name = secure_filename(file.filename)
        file_path = op.join(app.config['IMAGE_UPLOAD_FOLDER'], file_name)
        file.save(file_path)
        return file_path
    return 'no_allowed_file', 415  # Unsupported Media Type

As you can see, we check the input and image type, save it to a static folder served by the application using filename attribute, then return the relative image path.

Options

  • uploadImage: If set to true, enables the image upload functionality, which can be triggered by drag&drop, copy-paste and through the browse-file window (opened when the user click on the upload-image icon). Defaults to false.
  • imageMaxSize: Maximum image size in bytes, checked before upload (note: never trust client, always check image size at server-side). Defaults to 1024*1024*2 (2Mb).
  • imageAccept: A comma-separated list of mime-types used to check image type before upload (note: never trust client, always check file types at server-side). Defaults to image/png, image/jpeg.
  • imageUploadEndpoint: The endpoint where the images data will be sent, via an asynchronous POST request. The server is supposed to save this image, and if it's successful, return a 200-OK HTTP response containing the relative path of the image. No default value.
  • imageTexts: Several string literals used in image-upload features:
    • errorImageTooBig: The error prompted to the user when the size of the image being imported is bigger than the imageMaxSize, where #image_name#, #image_size# and #image_max_size# will replaced by their respective values. Defaults to Image #image_name# is too big (#image_size#).\n' + 'Maximum file size is #image_max_size#..
    • sizeUnits: A comma-separated list of units used to display messages with human-readable file sizes. Defaults to b,Kb,Mb.

TODO

I'm considering to support copy-paste and drag&drop, among with minor improvements and bugfixes.

Reviews and suggestions are welcome. :)

@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 5, 2019

Just added drag&drop:

pr-upload-image-drag drop

Also fixed a bug, now a popup is displayed when the image is too big:

image

@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 5, 2019

Added status messages, similarly to GitHub text areas (only displayed if uploadImage option is set to true):

pr-upload-image-status-bar

Also added a new EasyMDE function, updateStatusBar(itemName, content): this allows to edit any items in the status bar with custom triggers (not only cm.on('change')).

Not sure but it can be a solution to avoid the uniqueId problem to update the autosave status, because autosave function could just call updateStatusBar instead of accessing the auto-save status with getElementById.

@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 6, 2019

Aaand... copy-paste is here! 😄

pr-upload-image-copy-paste

@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 7, 2019

I think it's good for a proof of concept, I will wait for feedback before to continue.

@Ionaru can I have your opinion and/or some reviews about this PR? :)

roipoussiere added some commits Mar 7, 2019

@Ionaru

This comment has been minimized.

Copy link
Owner

Ionaru commented Mar 11, 2019

From your videos the feature looks awesome! I'm sorry I haven't been able to give it a closer look yet, things have been pretty busy for me.
I will try to have a better look at your feature this week.

@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 13, 2019

With this last commit we can call openBrowseFileWindow() in order to use it outside the textArea:

document.getElementById('myImage').addEventListener('click', function(event) {
    easyMde.openBrowseFileWindow(function(imagePath) {
        console.log('image path: ' + imagePath);
    });
});

In this example, a click on myImage will open the browse file window, then save the images after validation, but instead of adding markdown on the text-area, it prints the image path.

@Ionaru
Copy link
Owner

Ionaru left a comment

I've been testing with this code enabled locally and have a few points:

  • When dragging an image into the editor and not dropping it, but cancelling the drop, the status bar text does not go back to sbInit but stays at sbOnDragEnter.
  • When an error happens when uploading an image (rejected or 404), the status bar text stays at "Uploading ... 100%" instead of going back to sbInit or saying what has happened.
  • This whole system uses a lot of top-level API keys, would it be possible to group a few under imageUpload or something that accepts an object, true for defaults and false to disabled it? I believe other parts of the API work like this as well.
@roipoussiere

This comment has been minimized.

Copy link
Author

roipoussiere commented Mar 29, 2019

Thanks for testing. I will fix points 1 and 2 next week.

This whole system uses a lot of top-level API keys, would it be possible to group a few under imageUpload or something that accepts an object, true for defaults and false to disabled it? I believe other parts of the API work like this as well.

I agree. I suggest:

options
├── ...
└── uploadImage
    ├── enableUpload
    ├── imageMaxSize
    ├── imageAccept
    └── imageUploadEndpoint

imageTexts location is related to #68: I would personally prefer a top-level texts (or similar), because imageUpload is not the only feature that need to allow text replacement. What do you think about:

options
├── ...
└── texts
    ├── ...
    └── imageTexts
        ├── sbInit
        ├── sbOnDragEnter
        ├── sbOnDrop
        ├── sbProgress
        ├── sbOnUploaded
        └── sizeUnits

And then, for the next version of easyMde, add, little by little, other texts in this node.

Same for errorMessages, it could be:

options
├── ...
└── errorMessages
    ├── ...
    └── uploadImagesErrors
        ├── noFileGiven
        ├── imageTypeNotAllowed
        ├── imageTooLarge
        └── imageImportError

And errorCallback could also be at the top-level, to allow other parts of easyMde a better integration with the parent base code.

@kolaente

This comment has been minimized.

Copy link

kolaente commented Apr 14, 2019

How is the status here? We could really use this in Gitea.

@Ionaru

This comment has been minimized.

Copy link
Owner

Ionaru commented Apr 15, 2019

With the basic functionality working, but with a few minor points to solve, I expect this feature to be released in not too long a timeframe.

However with this being a non-profit open-source project, everyone that has contributed to this project has done so voluntarily in their own free time. As free time can be limited and is variable, I cannot guarantee a date when this feature will go live.

@kolaente

This comment has been minimized.

Copy link

kolaente commented Apr 16, 2019

@Ionaru sure, no pressure. Gitea itself is only built by volunteers, so I know that situation pretty good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.