Vanilla Staticman JS & HTML reorganization

[VincentTam]

Description

1. HTML reorganization:

   <article>
   <div class="post">
   </div>
   <div class="comments-container">
       <article class="comment" data-reply-thread="{{ $threadID }}">
       </article>
       <article class="comment">
           <article class="comment comment-reply" data-reply-thread="{{ $threadID }}">
           </article>
       </article>
   </div>
   </article>

   In my original PR for Staticman's nested comment support Staticman nested comments support from Huginn #69, I attempted to stored the post's $threadID with a hidden <span>—that's not the HTML5 way. I wish I knew the use of data- attributes earilier.

   hugo-future-imperfect-slim/layouts/post/staticman.html

   Lines 72 to 83 in 118943a

   	<div class="post-comment-timestamp">
   	<a href="#{{ ._id }}" title="Permalink to this comment">
   	<time datetime="{{ .date }}">{{ dateFormat "02/01/2006" .date }}</time>
   	</a>
   	</div>
   	<div class="post-comment-content">
   	{{ .body | markdownify }}
   	</div>
   	<div class='post-comment-reply-btn-container'>
   	<span class='post-comment-threadID hidden'>{{ ._id }}</span>
   	<a class='post-comment-reply-btn button' href='#post-js-form'>{{ i18n "reply" }}</a>
   	</div>

   Each comment is wrapped by an <article> tag with class name comment and id received from the data file. Each comment reply is nested inside a main comment. I've added the class comments-container to the <div> for JS event delegation (for details, see next point). I don't touch the commented TODO introduced in Reduce Theme Specificity #154 as I don't know much about the look yet—with the fixed JavaScript, the retrieval of reply target info should be OK.

   I changed the tag name surrounding the Hugo code for Disqus from <article> to <div>.

   I observed that the styles are controlled by the post class in the CSS, so changing the tag name to article should be no impact on the styles.

   The quotes for HTML syntax in layout/_defaults/comments.html are unified to single quotes. I left the double quotes in the Hugo code untouched.

2. assets/js/staticman.js in Vanilla JS

   • jQuery syntax for HTML element selection $.(...) replaced by document.querySelector(...) in most cases.

   • jQuery method ready() replaced by self-executing JS function (function(){...})();

   • some other method translation in the same sense: val(), addClass(), etc.

   • event delegation: Vanilla JS's addEventListner(evt, handler) method only allows adding listener to one single HTML element at a time, while jQuery allows adding that to a class of HTML elements $('comments').on(evt, handler). To write readable and maintainable code, I favored adding a new CSS class "comments-container" in layouts/_default/comments.html rather than using JS code to navigate the DOM structure like form.parentNode.nextSibling, which can be easily broken in case that some siblings are inserted in between.

     hugo-future-imperfect-slim/assets/js/staticman.js

     Lines 96 to 104 in 21d42da

     	// record reply target when one of the "reply" buttons is pressed
     	document.querySelector('.comments-container').addEventListener('click', function (evt) {
     	let target = evt.target;
     	if (target.matches('.comment-reply-btn')){
     	resetReplyTarget();
     	let cmt = target;
     	while (!cmt.matches('.comment')) { // find the comment containing the clicked "reply" button
     	cmt = cmt.parentNode;
     	}

   • jQuery method ajax() replaced with XHR.

     hugo-future-imperfect-slim/assets/js/staticman.js

     Lines 35 to 49 in 21d42da

     	let xhr = new XMLHttpRequest();
     	xhr.open('POST', url);
     	xhr.setRequestHeader('Content-Type', 'application/json;charset=UTF-8');
     	xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
     	xhr.onreadystatechange = function () {
     	if (xhr.readyState === XMLHttpRequest.DONE) {
     	let status = xhr.status;
     	if (status >= 200 && status < 400) {
     	formSubmitted();
     	} else {
     	formError(err);
     	}
     	}
     	};
     	xhr.send(formData);

   • jQuery method serialize() for converting form data to URL encoded string replaced with JSON-friendly string.

     hugo-future-imperfect-slim/assets/js/staticman.js

     Lines 17 to 32 in 21d42da

     	// Convert form fields to a JSON-friendly string
     	let formObj = Object.fromEntries(new FormData(form));
     	let xhrObj = {fields: {}, options: {}};
     	Object.entries(formObj).forEach(([key, value]) => {
     	let a = key.indexOf('['), b = key.indexOf('reCaptcha');
     	if (a == -1) { // key = "g-recaptcha-response"
     	xhrObj[key] = value;
     	} else if (a == 6 || (a == 7 && b == -1)) { // key = "fields[*]", "options[*]"
     	xhrObj[key.slice(0, a)][key.slice(a + 1, -1)] = value;
     	} else { // key = "options[reCaptcha][*]"
     	// define xhrObj.options.reCaptcha if it doesn't exist
     	xhrObj.options.reCaptcha = xhrObj.options.reCaptcha || {};
     	xhrObj.options.reCaptcha[key.slice(b + 11, -1)] = value;
     	}
     	});
     	let formData = JSON.stringify(xhrObj); // some API don't accept FormData objects

     I've hard-coded numerical values 6, 7 and 11 representing the length of fields, options and reCaptcha][ respectively for substring extraction from form input fields' name like fields[email] and options[reCaptcha][siteKey]. The goal is to construct a JSON-friendly string like Understand MISSING_PARAMS eduardoboucas/staticman#412.

   • replaced all instances of var to let to limit the scope of the variables to inside the function only: to avoid the following variables, espacially url and api, from overwriting variables in other files with the same name.

     hugo-future-imperfect-slim/assets/js/staticman.js

     Lines 9 to 15 in 21d42da

     	// Construct form action URL form JS to avoid spam
     	let api = '{{ .api }}';
     	let gitProvider = '{{ .gitprovider }}';
     	let username = '{{ .username }}';
     	let repo = '{{ .repo }}';
     	let branch = '{{ .branch }}';
     	let url = ['https:/', api, 'v3/entry', gitProvider, username, repo, branch, 'comments'].join('/');

Motivation and Context

It's motivated by the goal to replace jQuery with Vanilla in #231 (comment). This resolves #242 and resolves #243.

Screenshots (if appropriate):

[You can use Windows+Shift+S or Control+Command+Shift+4 to add a screenshot to your clipboard and then paste it here.]

Example

• demo site: https://vincenttam.frama.io/fish-demo/
• source: https://framagit.org/VincentTam/fish-demo
• GitLab bot's commit: https://framagit.org/VincentTam/fish-demo/-/commit/d851332

Checklist:

• I have updated the documentation, as applicable.
• I have updated the theme.toml, as applicable.

Critique

The JS code is repetitive. From an OO point of view, directly changing the state of the HTML element should give more concise JS code, and the l10n of button texts can be stored in CSS files. I'm not sure if {{ i18n ... }} can be called in assets/css/*.css. Even though that's possible, Hugo experts might prefer putting UI text in the Go-HTML template layout/**/*.html.

hugo-future-imperfect-slim/assets/js/staticman.js

Lines 66 to 78 in 21d42da

	function showAlert(msg) {
	if (msg == 'success') {
	form.querySelector('.submit-notice').classList.add('submit-success')
	form.querySelector('.submit-success').classList.remove('hidden'); // show submit success message
	form.querySelector('.submit-failed').classList.add('hidden'); // hide submit failed message
	} else {
	form.querySelector('.submit-notice').classList.add('submit-failed')
	form.querySelector('.submit-success').classList.add('hidden'); // hide submit success message
	form.querySelector('.submit-failed').classList.remove('hidden'); // show submit failed message
	}
	form.querySelector('input[type="submit"]:enabled').classList.remove('hidden'); // show "submit"
	form.querySelector('input[type="submit"]:disabled').classList.add('hidden'); // hide "submitted"
	}

[pacollins]

Haven't forgotten. Should have more free time in the coming weeks!

[pacollins]

Initial review looks great - all of your comments seem to be good and logical. I will start a full review this evening.

[pacollins]

I assume that this is the portion that you want to redo to clean up that block of the JS.

hugo-future-imperfect-slim/layouts/_default/comments.html

Lines 53 to 57 in 21d42da

	{{/* Start comment form alert messaging */}}
	<div class='submit-notice'>
	<strong class='submit-notice-text submit-success hidden'>{{ i18n "success_msg" }}</strong>
	<strong class='submit-notice-text submit-failed hidden'>{{ i18n "error_msg" }}</strong>
	</div>

If this is true (and the only thing), then I don't feel that is necessary for this PR. Unless I am misunderstanding the problem, I don't see a way to reduce the redundancy. Either we now have language-specific CSS files (since i18n would have to take effect at build), or we are still stuck with both options in the HTML so it builds and can run i18n.

A possible solution to clean it up is using msg from the if statement below to build the query selector.

hugo-future-imperfect-slim/assets/js/staticman.js

Lines 66 to 78 in 21d42da

	function showAlert(msg) {
	if (msg == 'success') {
	form.querySelector('.submit-notice').classList.add('submit-success')
	form.querySelector('.submit-success').classList.remove('hidden'); // show submit success message
	form.querySelector('.submit-failed').classList.add('hidden'); // hide submit failed message
	} else {
	form.querySelector('.submit-notice').classList.add('submit-failed')
	form.querySelector('.submit-success').classList.add('hidden'); // hide submit success message
	form.querySelector('.submit-failed').classList.remove('hidden'); // show submit failed message
	}
	form.querySelector('input[type="submit"]:enabled').classList.remove('hidden'); // show "submit"
	form.querySelector('input[type="submit"]:disabled').classList.add('hidden'); // hide "submitted"
	}

Something like

if (msg == 'success') {
var clear = 'failed' }
else {
var clear = 'success'}

form.querySelector(',submit-notice).classList.add('submit-'+msg);
form.querySelector(`submit-`+msg).classList.remove('hidden');
form.querySelector('submit-'+clear).classList.add(hidden);

Obviously not optimized, but a potential approach.

[pacollins]

Looking at #178 again I came across fancyapps/fancybox#2581 - Looks like a Vanilla JS version of Fancybox should be ready soon.

[VincentTam]

If this is true (and the only thing), then I don't feel that is necessary for this PR. Unless I am misunderstanding the problem, I don't see a way to reduce the redundancy.

Agreed and thank you for your thoughts. The "critique" is just like the antithesis and synthesis combined.

With the use of custom data- attributes in HTML, a more "OO-friendly" approach would be:

• use only one <strong class="submit-notice" data-success_msg="{ i18n "success_msg" }" data-failed_msg="…"> tag.
• CSS classes submit-failed and submit-success would be kept for readability of the CSS file
• create an immutable enumeration const ApiResponseEnum = {success: 1, failed: 0}
• update CSS classes in the showAlert(msg) method
• add an event listener for CSS class change of the submit-notice

The last two points would involve playing with the strings "success" and "failed" in JavaScript. That wouldn't simplify the code. More lines would be added—IMHO, my current "redundant" approach is more readable to amateurs.