Proposal: Feature: Markdown preview for markdown-supported fields #260

Open
Francesco Bellini wants to merge 2 commits from Francesco-Bellini/extensions-website:markdown-preview into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
5 changed files with 123 additions and 2 deletions

View File

@ -0,0 +1,78 @@
const activeTabCls = "is-active"
Review

I'd suggest to not abbreviate this, but use activeTabClass instead. There is a missing ; at the end of the line.

I'd suggest to not abbreviate this, but use `activeTabClass` instead. There is a missing `;` at the end of the line.
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}`);
Review

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.

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>`;
}
Review

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>`;
}
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);
}
Review

Following the same principle here, I'd suggest to rename the nothingToPreview function to something like showEmptyState, or similar.

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)
}
});
}
}

View File

@ -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>
Review

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.

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...' %}"
Review

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.

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 %}

View File

@ -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']

View File

@ -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/',

View File

@ -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})