Proposal: Feature: Markdown preview for markdown-supported fields #260
78
common/static/common/scripts/markdown-preview.js
Normal file
@ -0,0 +1,78 @@
|
||||
const activeTabCls = "is-active"
|
||||
|
||||
|
||||
function initMarkdownPreview(fieldName, texts) {
|
||||
const field = document.querySelector(`textarea.form-control[name="${fieldName}"]`);
|
||||
const markdown = document.querySelector(`#markdown-preview-${fieldName}`);
|
||||
const tokenInput = document.querySelector(`input[name=csrfmiddlewaretoken]`)
|
||||
const writeTab = document.querySelector(`#tab-write-${fieldName}`);
|
||||
const previewTab = document.querySelector(`#tab-preview-${fieldName}`);
|
||||
const write = document.querySelector(`#write-${fieldName}`);
|
||||
const preview = document.querySelector(`#preview-${fieldName}`);
|
||||
Márton Lente
commented
I'd suggest radically simplifying JS selectors wherever possible. For example, for For overall class and variable naming, I'd recommend adopting a more granularized approach, starting with the less specific item. For example, instead of I'd suggest radically simplifying JS selectors wherever possible. For example, for `field` constant, instead of `document.querySelector('textarea.form-control[name="${fieldName}"]');` we could use something like `document.querySelector('.js-textarea-${fieldName}');`. This approach offers better performance and code readability, ideally using direct class selectors with minimal specificity and traversal.
For overall class and variable naming, I'd recommend adopting a more granularized approach, starting with the less specific item. For example, instead of `previewTab` and `writeTab`, we should use `tabPreview` and `tabWrite`. This way we could also alphabetically order constant definitons for better code formatting, while having the related items close together.
|
||||
let lastText; // To avoid calling multiple times unchanged text
|
||||
|
||||
function toggleActiveTab() {
|
||||
writeTab.classList.toggle(activeTabCls);
|
||||
previewTab.classList.toggle(activeTabCls);
|
||||
}
|
||||
|
||||
function opacity_75(text) {
|
||||
return `<span class="opacity-75">${text}</span>`;
|
||||
}
|
||||
Márton Lente
commented
I'd suggest to use the action name in the function name (as we do with other functions). Something like this could read better:
I'd suggest to use the action name in the function name (as we do with other functions). Something like this could read better:
```
function reduceOpacity(text) {
return `<span class="opacity-75">${text}</span>`;
}
```
|
||||
|
||||
function nothingToPreview() {
|
||||
markdown.innerHTML = opacity_75(texts.nothing_to_preview);
|
||||
}
|
||||
|
||||
Márton Lente
commented
Following the same principle here, I'd suggest to rename the Following the same principle here, I'd suggest to rename the `nothingToPreview` function to something like `showEmptyState`, or similar.
|
||||
function switchTabs(on, off) {
|
||||
on.style.display = "block";
|
||||
off.style.display = "none";
|
||||
toggleActiveTab();
|
||||
}
|
||||
|
||||
if (previewTab && field) {
|
||||
previewTab.addEventListener("click", function (e) {
|
||||
e.preventDefault();
|
||||
if (!previewTab.classList.contains(activeTabCls)) {
|
||||
switchTabs(preview, write)
|
||||
if (field.value !== lastText) {
|
||||
markdown.innerHTML = opacity_75(texts.loading);
|
||||
lastText = field.value;
|
||||
if (field.value.trim()) {
|
||||
fetch("/markdown/", {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"X-CSRFToken": tokenInput.value,
|
||||
},
|
||||
body: new URLSearchParams({
|
||||
text: field.value,
|
||||
}),
|
||||
})
|
||||
.then((response) => response.json())
|
||||
.then((data) => {
|
||||
const preview_markdown = data.markdown;
|
||||
if (preview_markdown.trim()) {
|
||||
markdown.innerHTML = preview_markdown;
|
||||
} else {
|
||||
nothingToPreview();
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
lastText = undefined;
|
||||
markdown.innerHTML = opacity_75(texts.error);
|
||||
});
|
||||
} else {
|
||||
nothingToPreview();
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
if (writeTab) {
|
||||
writeTab.addEventListener("click", function (e) {
|
||||
e.preventDefault();
|
||||
if (!writeTab.classList.contains(activeTabCls)) {
|
||||
switchTabs(write, preview)
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
@ -1,7 +1,8 @@
|
||||
{% load common %}
|
||||
{% load i18n common %}
|
||||
{% spaceless %}
|
||||
{% with type=field.field.widget.input_type classes=classes|default:"" placeholder=placeholder|default:"" %}
|
||||
{% with field=field|remove_cols_rows|add_classes:classes|set_placeholder:placeholder %}
|
||||
{% with is_markdown=field.name|is_markdown_field %}
|
||||
{% autoescape off %}
|
||||
{% firstof label field.label as label %}
|
||||
{% firstof help_text field.help_text as help_text %}
|
||||
@ -28,6 +29,16 @@
|
||||
</label>
|
||||
{% endif %}
|
||||
|
||||
{% if is_markdown and not field.is_hidden %}
|
||||
<div class="hero-tabs mb-2">
|
||||
<a id="tab-write-{{field.name}}" href="#write-{{field.name}}" class="is-active">{% trans 'Write' %}</a>
|
||||
<a id="tab-preview-{{field.name}}" href="#preview-{{field.name}}">{% trans 'Preview' %}</a>
|
||||
</div>
|
||||
<section id="preview-{{field.name}}" style="display: none;border-bottom: thin solid rgba(255, 255, 255, 0.1);">
|
||||
<div id="markdown-preview-{{field.name}}" class="style-rich-text px-3 pt-2 pb-3">{% trans 'Loading...' %}</div>
|
||||
</section>
|
||||
Márton Lente
commented
It's good to reuse the existing It's good to reuse the existing `hero-tabs` component here, but as we use it only against dark background elsewhere, its styling doesn't handle the light theme yet. We should fix its contrast and styling if the light theme is active.
|
||||
{% endif %}
|
||||
|
||||
{% if icon %}
|
||||
<div class="input-group">
|
||||
<div class="input-group-prepend">
|
||||
@ -36,7 +47,9 @@
|
||||
{{ field }}
|
||||
</div>
|
||||
{% else %}
|
||||
<section id="write-{{field.name}}">
|
||||
{{ field }}
|
||||
</section>
|
||||
{% endif %}
|
||||
{% endif %}
|
||||
|
||||
@ -47,6 +60,22 @@
|
||||
{% if field.errors %}
|
||||
<div class="invalid-feedback">{{ field.errors }}</div>
|
||||
{% endif %}
|
||||
|
||||
{% block scripts %}
|
||||
{% if is_markdown and not field.is_hidden %}
|
||||
<script>
|
||||
document.addEventListener('DOMContentLoaded', function() {
|
||||
initMarkdownPreview("{{field.name}}", {
|
||||
loading: "{% trans 'Loading...' %}",
|
||||
error: "{% trans 'Error getting preview...' %}",
|
||||
nothing_to_preview: "{% trans 'Nothing to preview...' %}"
|
||||
Márton Lente
commented
With the same granularized naming approach proposed above, the options to pass in could be named With the same granularized naming approach proposed above, the options to pass in could be named `msgLoading`, `msgError` and `msgEmptyState` to provide context and show we pass in strings here.
|
||||
})
|
||||
})
|
||||
</script>
|
||||
{% endif %}
|
||||
{% endblock scripts %}
|
||||
|
||||
{% endwith %}
|
||||
{% endwith %}
|
||||
{% endwith %}
|
||||
{% endspaceless %}
|
||||
|
@ -230,3 +230,7 @@ def to_int(value):
|
||||
return int(value)
|
||||
except (ValueError, TypeError):
|
||||
return 0
|
||||
|
||||
@register.filter
|
||||
def is_markdown_field(name: str) -> bool:
|
||||
return name in ['description', 'release_notes', 'message']
|
||||
|
@ -21,6 +21,7 @@ urlpatterns = [
|
||||
name='api-internal',
|
||||
),
|
||||
# Public API
|
||||
path('markdown/', api.MarkdownRenderApi.as_view(), name='markdown-rendering'),
|
||||
path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'),
|
||||
path(
|
||||
'api/v1/extensions/<str:extension_id>/versions/upload/',
|
||||
|
@ -4,6 +4,7 @@ from django.core.exceptions import ValidationError
|
||||
from django.db import transaction
|
||||
from django.http import JsonResponse
|
||||
from django.utils.decorators import method_decorator
|
||||
from django.views import View
|
||||
from django.views.decorators.cache import cache_page
|
||||
from drf_spectacular.utils import OpenApiParameter, extend_schema
|
||||
from rest_framework.permissions import AllowAny
|
||||
@ -13,6 +14,7 @@ from rest_framework.views import APIView
|
||||
from rest_framework.permissions import IsAuthenticated
|
||||
|
||||
from common.compare import is_in_version_range, version
|
||||
from common.markdown import render as render_markdown
|
||||
from extensions.models import Extension, Platform
|
||||
from extensions.utils import clean_json_dictionary_from_optional_fields
|
||||
from files.forms import FileFormSkipAgreed
|
||||
@ -278,3 +280,10 @@ def extensions_awaiting_review(request):
|
||||
}
|
||||
)
|
||||
return JsonResponse(response, safe=False)
|
||||
|
||||
class MarkdownRenderApi(View):
|
||||
|
||||
def post(self, request, *args, **kwargs):
|
||||
text = request.POST.get('text')
|
||||
rendered_markdown = render_markdown(text)
|
||||
return JsonResponse({'markdown': rendered_markdown})
|
||||
|
I'd suggest to not abbreviate this, but use
activeTabClass
instead. There is a missing;
at the end of the line.