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"
|
||||
Francesco-Bellini marked this conversation as resolved
|
||||
|
||||
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>`;
|
||||
}
|
||||
Francesco-Bellini marked this conversation as resolved
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);
|
||||
}
|
||||
|
||||
Francesco-Bellini marked this conversation as resolved
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.
Márton Lente
commented
@Francesco-Bellini see the screenshot what I mean here for reference: So that the component's colours don't work when we switch to light theme (as we haven't used the component against light backgrounds yet). @Francesco-Bellini see the screenshot what I mean here for reference:
![Screenshot_20241128_171858.png](/attachments/d0444fdd-f0e6-4e88-9348-29c597c9127d)
So that the component's colours don't work when we switch to light theme (as we haven't used the component against light backgrounds yet).
Francesco Bellini
commented
Yes, marton, sorry for the misunderstanding, my reaction only ment "Good catch!" I always use dark mode, and sometimes i forgot to consider the light one! I'll apply the needed adjustments 👍 Yes, marton, sorry for the misunderstanding, my reaction only ment "Good catch!"
I always use dark mode, and sometimes i forgot to consider the light one!
I'll apply the needed adjustments 👍
|
||||
{% 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...' %}"
|
||||
Francesco-Bellini marked this conversation as resolved
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']
|
||||
|
I'd suggest to not abbreviate this, but use
activeTabClass
instead. There is a missing;
at the end of the line.