[Discussion] Refactor Comment Editor Using OOP

[noi5e]

I wanted to open up an issue to collect ideas/observations I've had about the comment editor's state management. I'll leave this issue open for a few weeks to collect thoughts as they come up (if that's okay). I'm hoping this will help us to conceptualize a code overhaul.

To start with, I wanted to give a bug as an example:

Video of this bug in action

1. Go to a note/question/wiki page with multiple comments.
2. Open up a "Reply to this comment..." form.
3. Open up a separate Edit Comment form.
4. Scroll back to the Reply Comment form.
5. Press rich-text buttons in the Reply Comment form.

✅ Expected Behavior: Rich-text changes go in the Reply Comment form.
❌ Actual Behavior: They go in the Edit Comment form.

Why This Happens

It's because we set state (for the focused comment form) here in the edit comment form partial:

plots2/app/views/comments/_edit.html.erb

Lines 46 to 52 in a3c8796

	function setInit(id) {
	var args = {
	'textarea': 'c'+id+'text',
	'preview': 'c'+id+'preview'
	}
	$E.initialize(args);
	}

But we don't do a similar thing when we open a reply comment form:

plots2/app/views/notes/_comment.html.erb

Lines 213 to 217 in a3c8796

	<script>
	$("#comment-<%= comment.cid %>-reply-toggle").click(function () {
	$("#comment-<%= comment.cid %>-reply").toggle();
	});
	</script>

The Bug Fix

I currently have a PR open that fixes the above bug at #9003.

I intentionally decided not to fix the bug by putting lines to set state when opening a reply comment form.

After all, just because a user opens up a comment form, doesn't mean that they necessarily want the focus to be there.

I thought a better fix would be to set state when the rich-text button was clicked. That seems to signal stronger user intention.

State Management Idea

So the idea is to start thinking about which user actions should set state, and which should not.

✅ Set State

• click on toolbar buttons
• drag-and-drop an image
• type text into a form
• load a new page (initial state should focus on the main comment form)

❌ Don't Set State

• open a reply form
• open an edit form

The Goal

Just to summarize: some sort of object-oriented, React-like refactoring where as much of this code can be inside a single script, creating modular comment form objects, where the state can be clearly transparent at each step of the way. 🎈

---

(This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)

[jywarren]

I wonder, once all system tests are in place, would an actual React component be plausible here? I don't have much experience with React, do you? I'm mainly just curious how integration would work and if it would be worth it. Curious to hear your thoughts! I like what you're planning on here, thanks!

[noi5e]

@jywarren I do have an okay amount of React experience. I see that there is a Rails React gem... It's worth looking into in a deeper way, but I have a feeling it would require rewriting the entire codebase in React, like v3 of the entire site. I've never heard of, for example, using React for just the comments form on a website... Maybe this does happen, maybe it doesn't, but I'm only familiar with entire sites built with React.

I guess I have a bias towards React being a junior developer, and it being the framework I'm most familiar with. I'd be willing to pitch in my two cents further down the line if Public Lab considered switching to a new stack!

Luckily, I think the Comment Editor is small enough that we can get somewhere without using React... Like, I'm working on a PR right now that I'll link here that's doing some object-oriented refactoring of editor.js, and I think it'll improve things a lot.

Another related issue I'm discovering in my refactoring is that a lot of the scripts used with the Comment Editor, like dragdrop.js and editor.js are repurposed on other places in the site, like the legacy editor in wiki/new, wiki/edit, and I just discovered features/new as well... That's turning out to be a bigger hurdle than state management.

[jywarren]

I believe it is possible to run just one component, and we've been interested in that as a proof of concept before trying to do the entire site... but really, that's out of scope unless you finish early and are really interested in it, so no worries!

I hear you re: reused scripts. I think it's possible to explore copying the scripts and splitting their use cases instead of trying to retain the code re-usability, if that is slowing down your work and/or refactoring progress!