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

Make it possible to customize image insertion code. #20

Merged
merged 2 commits into from May 14, 2016
Merged

Make it possible to customize image insertion code. #20

merged 2 commits into from May 14, 2016

Conversation

aaugustin
Copy link
Contributor

It can be useful to insert more that just a markdown image tag, for
example to provide additional control on the layout of images. Since
there's no convenient way to handle this on the JavaScript side, we
generate the code to insert in Python. Then it can be overridden with
the existing extensibility mechanism, that is, with a custom view.

It can be useful to insert more that just a markdown image tag, for
example to provide additional control on the layout of images. Since
there's no convenient way to handle this on the JavaScript side, we
generate the code to insert in Python. Then it can be overridden with
the existing extensibility mechanism, that is, with a custom view.
The previous commit is backwards-incompatible for users who pointed
MARKDOWNX_UPLOAD_URLS_PATH to a custom view. This commit restores
backwards-compatibility with the previous API.
@adi-
Copy link
Member

adi- commented May 5, 2016

I have some doubts about Markdown compatibility. As syntax states, it only supports tag. For now, any additional info is not supported (classes, ids, inlines). However, it is still doable by proper using of styling (css). What do you think?

@aaugustin
Copy link
Contributor Author

My use case is to insert {% embed_image "<image-slug>" "<position>" "<width>x<height>" %}. Markdown keeps it as is; then I put the result through Django's template engine to render appropriate HTML. I admit it's an atypical use case...

I believe a more common use case for customizing the upload function would be the ability to attach other files (e.g. PDF) and put a download link instead of an image embed.

@aaugustin
Copy link
Contributor Author

Another use case for this would be to use the "image reference" syntax:

![Alt text][id]

[id]: url/to/image  "Optional title attribute"

It's uncommon but legal!

@adi-
Copy link
Member

adi- commented May 5, 2016

My use case is to insert {% embed_image "" "" "x" %}. Markdown keeps it as is; then I put the result through Django's template engine to render appropriate HTML.
I believe a more common use case for customizing the upload function would be the ability to attach other files (e.g. PDF) and put a download link instead of an image embed.

As this is the right approach, I would add a general mechanism for adding files/links.

In my opinion, current ammend is not "pluggable". You just can't use custom image_code. You need to alter views.py first. I would add some simple mechanism to do that.

@adi-
Copy link
Member

adi- commented May 5, 2016

Fast idea: template file with attachment's type (content_type – string) like:

  • image – !()[]
  • pdf – ...
  • ... – ...

It could be simple editable for custom altering.

@aaugustin
Copy link
Contributor Author

aaugustin commented May 5, 2016

If I may take a step back for a minute, I think that django-markdownx already provides several levels of configurability, perhaps too many, and adding more may make the configuration more intimidating than practical :-)

There are three ways to change the markdown rendering:

  • MARKDOWNX_MARKDOWNIFY_FUNCTION
  • MARKDOWNX_MARKDOWN_EXTENSIONS + MARKDOWNX_MARKDOWN_EXTENSIONS_CONFIGS (but that doesn't allow selecting the output format)
  • pointing MARKDOWNX_URLS_PATH to a custom view

If I were designing a similar library from scratch, I'd just use reverse('markdownx_preview') as the preview URL and tell users to point a URL with that name to a function that does what they want. MarkdownifyView is two lines long; copy-pasting and customizing it isn't an issue.

There's only one way to customize image upload and insertion:

  • pointing MARKDOWNX_UPLOAD_URLS_PATH to a custom view and replacing ImageUploadView

I think that's good for flexibility (although I would have relied on URL reversing as shown above rather than on a setting).

If you want to make it easier for users to override smaller bits of functionality, you could restructure ImageUploadView and create two methods dedicated to handling the two pieces of functionality users may want to customize:

  • uploading the image
  • rendering the snippet that will be inserted into the markdown

However, designing a convention to select a template depending on the file type sounds (to me) more complicated than just inheriting a class-based view and replacing a method with my own logic. A little bit of code can be more convenient than configuration.


So, this is a pretty long comment, and I realize such large changes may be impractical... but I wanted to share my ideas about customisation :-)

Really, the only change I need is to make the JavaScript insert what the Django view returns as is. Hence this pull request!


To give you an idea my use case, here's how I overrode the views in my application:

# We replace markdownx's views entirely because they cannot be customized
# easily. Having all the code here makes it easier to see what's going on.

import os.path
import urllib.parse

from django import forms
from django.core import urlresolvers
from django.http import HttpResponse, JsonResponse
from django.utils.text import slugify
from django.views.generic import View

from ..models import Image
from .utils import markdownify


class MarkdownifyView(View):

    def post(self, request, *args, **kwargs):
        assert self.request.is_ajax()

        return HttpResponse(markdownify(request.POST['content']))


class UploadImageForm(forms.Form):

    image = forms.ImageField()


class ImageUploadView(View):

    def post(self, request, *args, **kwargs):
        assert self.request.is_ajax()

        # Super sketchy way to get the post id without touching markdownx.
        referer = request.META['HTTP_REFERER']
        parsed = urllib.parse.urlparse(referer)
        resolved = urlresolvers.resolve(parsed.path)
        assert resolved.view_name == 'admin:blog_post_change'
        post_id = resolved.args[0]

        # Minimal sanity check, simalar to what markdownx does.
        form = UploadImageForm(request.POST, request.FILES)

        if form.is_valid():
            picture = form.cleaned_data['image']
            name = os.path.splitext(picture.name)[0]
            slug = slugify(name)
            Image.objects.create(
                post_id=post_id,
                picture=picture,
                legend=name,
                alt_text=name,
                slug=slug,
            )
            image_code = '{{% blog_image "{}" "center" %}}'.format(slug)
            return JsonResponse({'image_code': image_code})

        else:
            return JsonResponse(form.errors, status=400)

@aaugustin
Copy link
Contributor Author

Hello @adi-, let me know if I can do anything to help move this forwards (and stop using my fork). Thanks :-)

@adi-
Copy link
Member

adi- commented May 13, 2016

I am little bit busy right now, but I am going to attach your amends in few days, I guess :)

@aaugustin
Copy link
Contributor Author

Great, thanks! No hurry, I just wanted to know if this was still on your radar.

@adi- adi- merged commit 7b012db into neutronX:master May 14, 2016
@adi-
Copy link
Member

adi- commented May 14, 2016

Done. django-markdownx v1.5 uploaded to pypi

@aaugustin
Copy link
Contributor Author

Heh, I don't even have to beg for release, then... Thanks!

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.

None yet

2 participants