Merge comments/edit & comments/form Partials #9183
Conversation
…form-wrapper to #comment-form-wrapper-edit
|
|
| <% if current_user && current_user.id == @node.uid %> | ||
| <span class="pull-right" style="color:#ccc;"> | ||
| <a target="_blank" href="https://publiclab.org/comment-templates">Add a template</a> | ||
| </span> |
There was a problem hiding this comment.
Unrelated change: I'm deleting the template functionality in this PR.
| class="comment-form" | ||
| <% if location != :edit %> | ||
| action="/comment/create/<%= @node.nid %>" | ||
| data-remote="true" |
There was a problem hiding this comment.
This data-remote="true" (along with comment.js) seems to play a crucial role in form submission in edit forms vs. the others.
Edit comment submissions refresh the page, while reply & main comment form submissions don't!
Funnily enough, this was something that was stumping me in this comment here way back a the beginning of the internship.
There was a problem hiding this comment.
Like, if you're just searching in VSCode for places where submitFormAjax and addComment are called, you don't find anything. For the longest time, I thought that these functions weren't actually being called.
But now I think they are! I think they work by attaching extra classes to particular comment forms, and jQuery.triggering events. This intrigues me because a similar thing could be done to edit comment forms so they are submitted with AJAX and don't refresh the page.
There was a problem hiding this comment.
OK, i think i can help here -- the data-remote attribute is used by the obscure rails-ujs library -- https://guides.rubyonrails.org/working_with_javascript_in_rails.html#remote-elements
However this is so obscure that I think it's just not worth using this technique; we don't have to remove it but let's add an explanatory comment to help future coders.
There was a problem hiding this comment.
Ahh, very good to know!
Codecov Report
@@ Coverage Diff @@
## main #9183 +/- ##
=======================================
Coverage ? 82.06%
=======================================
Files ? 100
Lines ? 5939
Branches ? 0
=======================================
Hits ? 4874
Misses ? 1065
Partials ? 0 |
|
Code Climate has analyzed commit 3f2e807 and detected 0 issues on this pull request. View more on Code Climate. |
| <!-- get_comment_form_id defined in app/helpers/application_helper.rb: --> | ||
| <% comment_form_id = get_comment_form_id(location, local_assigns[:reply_to], nil) %> | ||
| <div class="comment-form-wrapper card-body" style="background-color:#f8f8f8; border: 1px solid #e7e7e7;padding: 18px;"> | ||
| <form id="comment-form-<%= comment_form_id %>" class="comment-form" data-remote="true" action="/comment/create/<%= @node.nid %><%= "?type=question" if local_assigns[:type]=="question" %>" <% if local_assigns[:aid].blank? %>method="post"<% end %>> |
There was a problem hiding this comment.
Deleting this ?type=question part here.
|
Just did a lot of double-checking of the now-deleted edit partial. This one is ready to merge! |
|
This looks great. Thank you! This will also be easy to track if there are any errors, as the PR touches both files. So even if we catch something later this will be traceable. Thanks a lot!!! |
* merge comments/_edit & comments/_form partials * delete templates functionality * merge comments/_edit & comments/_form partials; change #edit-comment-form-wrapper to #comment-form-wrapper-edit * stop attaching ajax:event to edit forms * stop running scripts on edit forms * add hidden_field_tag for reply_to
* merge comments/_edit & comments/_form partials * delete templates functionality * merge comments/_edit & comments/_form partials; change #edit-comment-form-wrapper to #comment-form-wrapper-edit * stop attaching ajax:event to edit forms * stop running scripts on edit forms * add hidden_field_tag for reply_to
* merge comments/_edit & comments/_form partials * delete templates functionality * merge comments/_edit & comments/_form partials; change #edit-comment-form-wrapper to #comment-form-wrapper-edit * stop attaching ajax:event to edit forms * stop running scripts on edit forms * add hidden_field_tag for reply_to
These two partials are basically very close copies of each other... Even more so with all the recent refactoring of the Comment Editor.
In the past, if I made a change in one partial, I had to do it in the other to match.
Merging these two partials into one, to increase maintainability and DRYness!
(This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)