Plots2: [Discussion] Rewrite Comment Editor with React

[noi5e]

Hooray! The first part of the React rewrite of the Comment System was merged in #9176.

Currently, the React system can do CRUD of comments. React and webpacker were installed in this phase.

There's an issue open here for Phase 2, which will measure performance with system tests, and add further functionality like emoji reactions, comment preview, etc: #9365

Let's see how this wild experiment goes! I installed React & Webpack on a development branch, and I'm going to see if I can reproduce the Comment Editor using React.

Doing all my work in Draft PR #9176 and in the branch at react-comments

The purpose of this is to figure out:

1. Does React offer any significant advantage in terms of state management? As we've seen, Comment Editor state can get kind of complicated. And is this advantage worth it, weighing the cost-benefit of adding a new framework to the stack?
2. If this is a worthwhile project, what other parts of the site can be given this treatment?

Also Done in PR #9176

• [x] put static initialProps that don't change into a context provider
• [x] use ActionController::Base.helpers.<helper> to add view helpers to the controller
• [x] figure out a way (via JS or Rails's <%= raw %>) to encode comment text properly
• [x] undo nested comments state!
• [x] refactor CommentsContainer helper functions for reduced complexity
• [x] populate state formToggleState and textAreaValue when fresh comment is posted
• [x] refactor so setTextAreaValues isn't passed down 5-7 components
• [x] create CommentReplies in CommentsList, to avoid passing down props
• [x] wrap CommentsContainer in new App component
• [x] pass parameters to formSubmit and buttonClick functions (not event.target.dataset)
• [x] set up guest (not logged-in) browsing
• [x] reply form toggle displays link to login if no currentUser
• [x] create unique routes for CRUD functions, ie. /comment/REACT/create/46

---

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

[noi5e]

Installation went smoothly, which is nice! Here's what I have so far on the upstream/react-comments branch:

1. Visit any note (not question or wiki) page, and pass in the parameter react=true in the URL.
2. Hello React!!! is rendered instead of the comment editor!

[noi5e]

Lots of errors in CI for the draft PR #9176:

The errors refer to /public/packs-test/manifest.json, which exists, but is .gitignored locally. This probably doesn't exist in the testing environment, so that's what's causing the errors. This works fine locally however!

[noi5e]

This is coming along nicely. I have a list component that is able to iterate over an array of comment objects:

As you can see, the rendering is very weird with <p>comment text</p>. It probably has something to do with encoding when going from Rails -> JSON -> React.

Pre-Formatting State for React

From reading the docs for rails-react, I believe that these React components don't have access to Rails methods that views normally do (like node#method or order_by, etc.)

So we have to work around that by running all of our Rails methods on the state object before we feed it into the CommentsContainer component. That's what this code is doing:
https://github.com/publiclab/plots2/blob/ff75433297e13775353e4a4e9ca38abc78d85cac/app/views/notes/show.html.erb#L119-L140

(Don't worry, I will move this bulky block into application_helper.js when I'm done.)

This is kind of annoying to write this code, but it helps to anticipate the kind of state the views must have.

Server-Side Rendering

I don't know much about this aspect of React, but rails-react also has the option to render components server-side within the controller. See here and here. This would improve speed and performance a lot!

[noi5e]

Accessing comment.author

@jywarren Maybe you can help me here? I'm trying to write the React equivalent of the code here in the notes/_comment partial (looking up the author of the comment):
https://github.com/publiclab/plots2/blob/ff75433297e13775353e4a4e9ca38abc78d85cac/app/views/notes/_comment.html.erb#L18-L30

The problem is that the React state of comment looks like this in my app:

{
  "comments": [
    { 
      "comment": {
        "cid":2,
        "pid":0,
        "nid":46,
        "uid":1,
        "subject":"",
        "comment":"Woohoo, here's an awesome comment!",
        "hostname":"",
        "timestamp":1613426027,
        "status":1,
        "format":1,
        "thread":"01/",
        "name":null,
        "mail":null,
        "homepage":null,
        "aid":0,
        "comment_via":0,
        "message_id":null,
        "tweet_id":null,
        "reply_to":null,
        "flag":0},
      "commentText":"\u003cp\u003eWoohoo, here's an awesome comment!\u003c/p\u003e\n"
    }
  ],
  "commentsHeaderText": "Comments"
}

In other words, there isn't any author key present in the comment object.

Here's where the notes/_comment partial is rendered in the notes/_commentS partial:
https://github.com/publiclab/plots2/blob/ff75433297e13775353e4a4e9ca38abc78d85cac/app/views/notes/_comments.html.erb#L2-L13

1. comments is at first comments = @node.comments_viewable_by(current_user)
2. it's passed to the notes/_comment partial here: <%= render :partial => "notes/comment", :locals => {:comment => comment} %>
3. somehow comment.author is accessible here: <% if comment.author %>

I'm not able to access comment.author when doing the same things in React. What am I missing here? How is the author property of the comment node defined in Rails? Is it some kind of lookup between two database tables? And where is that defined exactly when comments is first defined as comments = @node.comments_viewable_by(current_user) in the notes/_comments partial above?

[jywarren]

OK, so i'm thinking on how to "pack" the association (which in this case is not a real relation but just a query for User.find(uid) in concerns/comments_shared.rb -- and the example i can think of is in MapKnitter:

https://github.com/publiclab/mapknitter/blob/3d8906e7f1babde58a8500c9d62895c7c7924be3/app/models/warpable.rb#L34-L49

This packs up whatever you want into a Hash format, which can then have .to_json run upon it:

https://github.com/publiclab/mapknitter/blob/3d8906e7f1babde58a8500c9d62895c7c7924be3/app/controllers/images_controller.rb#L36

Keep in mind this is a little more convoluted because in Ruby unlike JS we can't just assign arbitrary methods or attributes to an existing object, which is why we make a new object. So you might do something like (i think this should work):

{
  author => comment.author
}.to_json

Although there are probably other ways to do it too!

I'd be careful to try to do this only for the JSON response you're handing back to React, as it does add complexity and cost to the query if used in other scenarios where this wasn't deemed necessary. Are you doing it in a new controller action?

Hope this helps!

[noi5e]

@jywarren Thanks, yeah that is helpful! I think that will help once I definitively know what needs to be in the massive state object that Rails hands off to React. I'll probably write something similar.

I thought of another small question. Take a look at these lines I posted above from the new React revision:
https://github.com/publiclab/plots2/blob/ff75433297e13775353e4a4e9ca38abc78d85cac/app/views/notes/show.html.erb#L128-L129

I noticed that I wasn't able to set commentText by doing comment.commentText = "string". Hence why I call comment.as_json, which translates comment to a Hash (it starts off as an ActiveRecord object, which is immutable). Then merge, which adds a new key => value pair to the Hash. I'd like to know if there's a more practical way to do this. I'm still learning Ruby!

As you can see, there's a big up-front cost to making the massive state object before it's handed off to React. To answer your question, this isn't being done in the controller action, but in the view. I'm just getting this up and running first, then I'm sure it can easily be moved into the controller.

Just to reiterate, it's also possible to render all the components server-side, which would boost performance by a lot!

[jywarren]

Hmm, interesting, so what are you aiming to do with the line comment.merge({ "commentText" => comment_text })? this would be a change only after the comment is fetched from the database and before it's rendered to the view, but it's not a persistent change. You can overwrite comment.commentText but you need to then run comment.save (assuming comment is a full Ruby/ActiveRecord object) for it to save to the database. Otherwise your change is only to the in-memory copy of the record. Does that help?

[noi5e]

It's like this:

1. comment right out of the box is I believe an ActiveRecord object. I can read values from it, but I can't write anything to it (ie. comment[:commentText] = "string" returns error)
2. So that's why I transform it into a Hash object here: comment = comment.as_json
3. And then add a new key => value pair here: comment.merge({ "commentText" => comment_text })

I'm sure there's a much simpler way to do this, probably the MapKnitter example where I just construct the JSON object from scratch. Also this will all eventually end up in the controller so as not to complicate the views too much.

I just want to know what the best way to do this in Ruby is. I'm more used to JavaScript where you can willy-nilly add keys and values to objects, so this is a little new for me.

[jywarren]

Ok, do you want to permanently write to the record, so it's saved in the database, or just to the temporary copy in memory, and it's ok if the comment doesn't retain the change?

[noi5e]

The second! 😊 Definitely don't want to save anything to the database, I just want to package up everything that React needs in a JSON state object. This requires translating the ActiveRecord thingies into objects I can write values into.

[jywarren]

Yes then your code looks good! I had to do something similarly odd to achieve the same in the mapknitter code but there are multiple ways to do it and you're right that the hard part is getting around the inability t write arbitrary attributes in Ruby.

[jywarren]

And i don't know what the most legible or standard way of doing this is but since it looks a bit odd maybe we should leave a comment like "temporarily add in extra attribute to a copy of the record in memory" or something?

[noi5e]

Yeah, definitely going to make comments all over the place. After I make some decent progress, I'll move this all to the controller and just make a fresh Hash object to hold all the JSON. Similar to the MapKnitter example. I think that will be a lot easier to read.

[noi5e]

Coming along nicely! I am now generating initial React props in the notes#show controller, where they belong:
https://github.com/publiclab/plots2/blob/1fe4e518e342a7aff57e15d02d6d98660a5b9933/app/controllers/notes_controller.rb#L47-L68

However, some methods that the notes/_comment view uses are NOT available in the notes#show controller. So I iterate over the initial React props and add a few attributes to each comment:
https://github.com/publiclab/plots2/blob/1fe4e518e342a7aff57e15d02d6d98660a5b9933/app/helpers/application_helper.rb#L82-L93

As you can see, it's really weird that the comment text appears as <p>comment text</p>. I would like to find a way to encode these as proper HTML.

My next steps are adding basic state management. I'll work with the reply comment form and add a replyFormVisible: false prop that can toggle the form open and closed.

[jywarren]

Ah, i see - view helpers, makes sense - so we could also have the option to replace the time-related ones with JS-driven equivalents, but... a little harder to test efficiently i suppose. Although we'd be doing system tests anyways, right?

The extras, yeah, i don't see another way. Well, there are ways to use view helpers within a controller! It's:

ActionController::Base.helpers.<helper>

I think... Just in case that's helpful!

[jywarren]

As to the encoding, i bet it's being escaped at some level due to not being marked as safe HTML. In .erb files, you can use <%=raw ... %>, i wonder if there's an equivalent to stop escaping "unsafe" content in your workflow?

[jywarren]

One thing to think on here is how complex is the integration between the React client side code and the parent app. The fewer places we have to integrate the better; there should ideally be just a single "handoff point" where the thing is constructed, and all the params are being inserted.

Here's an example:

https://github.com/publiclab/plots2/blob/59299496d03dad4eac3ef8c0ff7d0b29c099c0e4/app/assets/javascripts/leafletHelper.js#L133-L142

Here we are modifying some of the options before handing them off, and we've wrapped it in a plots2-specific wrapper function. but we could just as easily have used a carefully configured constructor function directly in each place in the plots2 codebase.

[jywarren]

OK, i left a bunch of comments in your PR, hope they are helpful!!!

[noi5e]

ActionController::Base.helpers.

As to the encoding, i bet it's being escaped at some level due to not being marked as safe HTML. In .erb files, you can use <%=raw ... %>, i wonder if there's an equivalent to stop escaping "unsafe" content in your workflow?

Both super-helpful to know, thank you! Will revisit these later.

One thing to think on here is how complex is the integration between the React client side code and the parent app. The fewer places we have to integrate the better; there should ideally be just a single "handoff point" where the thing is constructed, and all the params are being inserted.

I hear you, and I think it looks pretty good so far. If I integrate the view helpers into the controller that 'bundles up' React state. That means there essentially is just one "handoff point." From that point forth, after the state is handed off to the React app, there's no more front-end interaction between the Rails parts and the React parts.

We'll see how this goes though. I have yet to do anything with form submission or make requests to the server (will start working on that soon), so yet to be seen what this is going to be like really.

[noi5e]

Pushed a few more commits, here's what it's looking like now:

State Management: Toggling Reply Form

I'm starting to work with state management a little more, using React Hooks. Basically, the Comment component reads the isReplyFormVisible state hook and renders the form accordingly. Video of this in action

This is how it's done in react-comments:
https://github.com/publiclab/plots2/blob/cf4ef1ea330512f5193aa8dfeb254bf5a1e6c372/app/javascript/components/Comment.js#L23-L24
https://github.com/publiclab/plots2/blob/cf4ef1ea330512f5193aa8dfeb254bf5a1e6c372/app/javascript/components/Comment.js#L56-L81

Versus how it's done in Rails, on the main branch:
https://github.com/publiclab/plots2/blob/cc98fd8a2b325fef7c7b7ab1dcc08ed83d76b04e/app/views/notes/_comment.html.erb#L166-L182
https://github.com/publiclab/plots2/blob/cc98fd8a2b325fef7c7b7ab1dcc08ed83d76b04e/app/views/notes/_comment.html.erb#L224-L228

I know it's a matter of preference, but I kind of prefer the React version for readability? 🤷

[noi5e]

React Context: Comment Toolbars

The other major change is state management for the comment toolbar. These are the buttons in the top-right corner of each Comment (not to be confused with the editor toolbar):

1. Edit Comment
2. Mark Comment as Spam
3. Delete Comment
4. Leave Emoji Reaction

These are deceptively complex buttons. Each one of them renders or doesn't render based on several conditionals. For example, the delete button only appears in these scenarios:

1. current_user created the note that is comment's parent
2. current_user is an admin or moderator
3. current_user created the comment

So I needed a way of feeding current_user into React as part of its initial props (remember that React can't access the Rails current_user helper). I chose to use React Context for this purpose. Basically React Context is a way of making currentUser and other variables available globally throughout the React app without having to pass variables down in the standard React way.

In other words, without React Context, I would have to pass currentUser from CommentsContainer => Comment => CommentToolbar => CommentToolbarButton, which is very tedious and potentially buggy. Much better to make it a global prop.

Here's the userinfo I bundle up in the notes/show controller for React's initial props:
https://github.com/publiclab/plots2/blob/cf4ef1ea330512f5193aa8dfeb254bf5a1e6c372/app/controllers/notes_controller.rb#L99-L112

[noi5e]

Next Steps

These commits mark a milestone. Basically, this work shows that React can render the Read part of CRUD and display comments from the server.

I'm going to start working on making interactive forms (which can be a little quirky in React). Then we'll see about form submission, which handles the Create, Update, and Delete parts.

If I can pull that off 🤞 then it's a reasonable bet that the rest of comment form functionality (rich-text changes & image upload, emoji reactions, etc.), are achievable as well. Also we can use comment form submission as a milestone to reflect on whether or not a React rewrite is worth pursuing or not.

[noi5e]

Today we have non-functional textarea and publish & preview buttons. Now to hook them up to React state, then after that AJAX form submission!

[noi5e]

The CommentForm component is now hooked up to React's state. So, typing in textareas updates state. I wrote a handleSubmit function that will console.log state when the Publish button is clicked.

Now to work on form submission! This is the first time the front-end is going to make requests to the server, which is exciting. Let's see how this goes, I might have to yarn install another library to make requests. Maybe axios?

[noi5e]

I had a go at having CommentsContainer do an jQuery AJAX request, and the results were... Weird and unexpected?

The request is successful. The comment gets inserted into the database. The weirdness begins when the Rails server sends back its 200 response. create.js.erb runs some JavaScript to insert the new comment into the DOM.

All of the old comments are still built with React. BUT the new comment is 100% completely functional because it's built with the notes/_comment Rails partial! It's a weird zombie hybrid of Rails + React!!!

VIDEO OF THE WEIRDNESS IN ACTION!

This shows me that we need to put a [:react] parameter in the /comment/create controller so that it just returns plain JSON, and not this weirdness. Just thought I'd leave a record because I found this interesting and amusing.

[noi5e]

Okay, so posting a comment, and posting a comment reply now works:

https://user-images.githubusercontent.com/4361605/109215768-7816db00-7768-11eb-8230-0f298e61f1de.mov

That takes care of C and R in CRUD (displaying comments, posting comments & comment replies). Now onto editing comments!

[noi5e]

😌 Whew, a lot of hard but satisfying work to get edit forms up and running. Here the latest updates:

• clicking on the pencil icon now toggles the Edit Comment form.
• Edit Comment form's <textarea> has a default value of rawCommentText
  
  
  
  • I had to do a lot of things behind the scenes with React state to make this work, which is typical of React & forms.
  
  

If the user edits the text, but cancels out of the form without publishing, I made the Edit Comment form's <textarea> value reset to the original rawCommentText. We don't have this feature in the current Comment Editor, but I decided to add it because I think it makes sense:
https://github.com/publiclab/plots2/blob/c9cd1dbb533231e4ec1480d4b3d6334ca0687487/app/javascript/components/Comment.js#L35-L44

https://user-images.githubusercontent.com/4361605/109436647-65eda480-79d5-11eb-9c9b-aa84dfd4d994.mov

The app is getting more and more complex with each change. I'm reducing complexity in each file by breaking things out into different components. As you can see, there's a LOT of components now:

This poses an organizational challenge. Soon, I'll have to put the components in different folders. Like, one folder Comment to hold all the components that make up an individual comment... AFAIK there aren't really any React file & folder organizing conventions that are set in stone. It's something that has to be figured out on the fly.

Finally I installed linters for React and React Hooks which are helping me be more confident about the code I'm writing!
https://github.com/publiclab/plots2/blob/c9cd1dbb533231e4ec1480d4b3d6334ca0687487/.eslintrc#L12-L17

[jywarren]

This is awesome to see, Will. Thanks for keeping the updates coming!!!

[noi5e]

@jywarren Thank you!! 🙏 If you get a chance, can you try merging the rails g commands? I tried to do it in the PR, but I think I don't have access to change Git workflows. See https://github.com/publiclab/plots2/pull/9176#issuecomment-784806141

[noi5e]

Finally figured out what was causing the comment HTML to not be displayed properly. Jeffrey was right in this comment https://github.com/publiclab/plots2/issues/9175#issuecomment-782149976, React doesn't set HTML in code to protect from browser attack.

So comments display properly now. Yay! 🎉

Taking care of some organizational things and tweaking some small things before I move on to updating comments (which I don't think will be too hard). Some other updates:

• Made a helpers.js file
• I had to fix posting comments and posting comment replies because they broke. It's a long story, but I had to make a helper function to deep clone arrays and objects because having React elements in state breaks the JSON.parse(JSON.stringify()) method of deep-cloning.
• Notifications flash on comment posting!

[noi5e]

Okay, editing comment forms now works!

It wasn't too hard, but this change exposes a couple of more complications that will take some more work to unravel 🥳

https://user-images.githubusercontent.com/4361605/109593756-69ad2400-7ac6-11eb-8f5a-2ab9698d7d76.mov

1. As you can see, the Edit Comment form remains open even after updating. This needs to change. It's not a simple fix, however. To do this, I have to restructure all the components so that the handleEditFormToggle function lives in CommentsContainer and not CommentToolbar. 🙌
2. The server JSON handling and state-updating functions are now absolute behemoths. I had to /* eslint-disable complexity */, but eventually I'm going to have to consolidate all this code and break it down into reusable helper functions. I'm going to finish DELETE comment functionality first, and then work on that afterward.

https://github.com/publiclab/plots2/blob/fc937b60162911fbd9a450e6b3f449939a74af04/app/javascript/components/CommentsContainer.js#L46-L128

[jywarren]

@jywarren Thank you!! 🙏 If you get a chance, can you try merging the rails g commands? I tried to do it in the PR, but I think I don't have access to change Git workflows. See #9176 (comment)

OK did this and didnt yet remove run: ./public/lib/.bin/webpack-dev-server but we can do that too if we need to, just ping me again!

[noi5e]

OK did this and didnt yet remove run: ./public/lib/.bin/webpack-dev-server but we can do that too if we need to, just ping me again!

Thank you! I don't think it's going to solve the weirdness in the PR right now, but I do have a feeling we're going to have to remove the webpack-dev-server part just because that's something I'm not doing locally.

[noi5e]

I moved the piece of state that handles whether or not the reply or edit forms are visible into the CommentsContainer component. This component is the highest-level component, so this means that I can do things like:

• toggle a comment form closed when a new comment is published
• do the same when a comment is updated

https://user-images.githubusercontent.com/4361605/109758588-a9483e80-7ba0-11eb-97aa-c219404c629d.mov

Creating, Reading, and Updating now are looking pretty natural and smooth with these new changes. Maybe more than the current Rails commenting system, which doesn't yet toggle forms shut upon submission!

Side-note, the state for comments is currently set up like so:

{ commentId: 1 }
{ commentId: 2, replies: { nestedComments }}

I'm finding out the hard way that this is not a good way to set up state! It requires a lot of recursion, and makes the code a lot less understandable. I have to remake state so that replies aren't nested within comments.

I feel like my brain is getting a huge workout trying to reason through the logic of passing props down, what needs to be done where, how to rewrite the components for legibility and maintainability... 🧠 😵 It's actually pretty fun and interesting, and maybe unique to React programming?

[jywarren]

Wow, pretty interesting. I am wondering if, towards the end (or even now if it would help your process?) it could be useful to do a Google Slides diagram of the different components, how they fit into one another, and what properties or actions each component has.

In the new version, would you pass a unique ID as a property to the comment for each reply, and separately fetch all replies, and re-thread them on receipt?

Thanks, @noi5e !!

[noi5e]

A few significant updates:

• I undid comment nesting for state that I mentioned here: https://github.com/publiclab/plots2/issues/9175#issuecomment-789455319
  
  
  
  • replies are now NOT nested within their parent comments
  
  
  • much less recursion everywhere, much less complication in terms of passing props down. for example, setTextAreaValues is now no longer passed down 5-7 components
  
  
  • the trade-off is that the CommentsList component became significantly more complicated, but it's still worth it
  
  
• when a fresh comment is posted, you can click to edit, and the edit comment form is populated with raw comment text
• wrapped CommentsContainer in a new App component
• broke out handleFormSubmit into handleCreateComment and handleUpdateComment functions
  
  
  
  • also started passing parameters into these two functions, instead of relying on e.target.dataset.formId (that's how the current Rails commenting system works) 😛
  
  

We're in good shape here! I feel good about merging this as is and then working on the new updates in separate PRs.

Going to work on my final Outreachy presentation for the rest of today, and also see about getting this working on unstable!

[jywarren]

This is really awesome, just wanted to say, thanks @noi5e!!!!