Add Unique IDs to Comment Forms #9062

noi5e · 2021-01-21T23:00:43Z

Related to #9004 (Object-Oriented Refactor of Comment Editor)

See this comment for more context: #9004 (comment)

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

gitpod-io · 2021-01-21T23:00:47Z

codecov · 2021-01-21T23:05:58Z

Codecov Report

❗ No coverage uploaded for pull request base (main@38e37eb). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #9062   +/-   ##
=======================================
  Coverage        ?   82.15%           
=======================================
  Files           ?      100           
  Lines           ?     5936           
  Branches        ?        0           
=======================================
  Hits            ?     4877           
  Misses          ?     1059           
  Partials        ?        0

noi5e · 2021-01-21T23:47:49Z

app/views/comments/_edit.html.erb

+        const textArea = 'c'+id+'text';
+        const preview = 'c'+id+'preview';
+        $E.setState(textArea, preview);
+      }



The changes in this file are mostly due to slightly fixing the indentation.

noi5e · 2021-01-21T23:48:43Z

app/helpers/application_helper.rb

-  # used in views/comments/_form.html.erb
-  def get_textarea_id(location, reply_to)
-    case location
-    when :main


deleting this function, it was unnecessary.

noi5e · 2021-01-21T23:50:13Z

app/views/comments/_edit.html.erb

-<div class="comment-form-wrapper">
-  <div class="card card-body bg-light">
-    <form method="post" id="c<%= comment.id %>edit" style="display:none;" class="well" <% if comment.is_a?Answer %> action= "/answers/update/<%= comment.id %>" <% else %> action="/comment/update/<%= comment.id %>" <% end %>>
+<div class="card card-body bg-light comment-form-wrapper">


I collapsed <div class="card card-body bg-light"> and <div class="comment-form-wrapper"> into one <div>. I know there's a lot of <div> spam in the views built up over the years, trying to do my part in reducing it.

I checked the CSS by both viewing locally and hunting through the stylesheets, and this doesn't seem to create any new issues.

…ain publiclab#9004

noi5e · 2021-01-22T00:48:23Z

app/views/editor/_editor.html.erb

@@ -17,7 +17,7 @@
  </div>
 </div>

-<div class="well" id="preview-main" style="display:none;"> </div>
+<div class="well" id="comment-preview-main" style="display:none;"> </div>



I know this is kind of weird on the wiki editor to have this ID be #comment-preview-main, but I have a master plan for this and will take care of it in the next few PRs.

Basically with an OOP refactor of editor.js, it will be possible to $E.initialize({default main editor form ID}).

…ain publiclab#9004

codeclimate · 2021-01-22T02:17:54Z

Code Climate has analyzed commit 698c8cf and detected 0 issues on this pull request.

View more on Code Climate.

noi5e · 2021-01-22T03:34:21Z

Okay, I kept trying to add more to this PR, but I've decided to stop here for now. @jywarren This one is ready to merge!

Just to summarize the changes I made in this PR:

MAIN comment form:
- <form id="comment-form> is now #comment-form-main
- #preview-main is now #comment-preview-main
REPLY comment form:
- <form id="comment-form> is now #comment-form-reply-123
- #preview1234 is now #comment-preview-reply-1234
EDIT comment form:
- <form id="c123edit"> is now #comment-form-edit-123

I'm basically just trying to get everything organized so that $E can setState on the last part of the IDs: main, or reply-1234 or edit-1234.

I still need to add unique IDs to _edit.html.erb (remember that it's distinct from _form.html.erb, which is what REPLY and MAIN uses):

#c1234textarea needs to be #text-input-edit-1234
#c1234preview needs to be #comment-preview-edit-1234

But I will do that in a separate PR!

jywarren · 2021-01-26T19:52:37Z

This looks great. Is it worth adding some extra CSS assertions to enforce the assumption of:

I'm basically just trying to get everything organized so that $E can setState on the last part of the IDs: main, or reply-1234 or edit-1234.

I still need to add unique IDs to _edit.html.erb (remember that it's distinct from _form.html.erb, which is what REPLY and MAIN uses):

That way it's actually written somewhere and the convention is documented and protected? CSS assertions are so cheap anyways...

jywarren

💪

Woohoo!

noi5e · 2021-01-26T20:40:58Z

@jywarren Thanks! I didn't know it was possible to hide whitespace changes. And I'll look into CSS assertions.

jywarren · 2021-01-26T21:00:20Z

yeah i thought it might be useful to know - it's a lifesaver when reviewing! Thank you!!

* delete get_textarea_id method publiclab#9004 * change #c23edit selector to #comment-form-edit-23 publiclab#9004 * assign unique IDs to comment forms publiclab#9004 * change selectors from #c1edit to #comment-form-edit-1 publiclab#9004 * change preview IDs from #preview1 to #comment-preview-1 publiclab#9004 * change main preview selector to #comment-preview-main from #preview-main publiclab#9004 * change main preview selector to #comment-preview-main from #preview-main publiclab#9004

noi5e added 3 commits January 21, 2021 14:53

delete get_textarea_id method publiclab#9004

4b0f4e4

change #c23edit selector to #comment-form-edit-23 publiclab#9004

5a0b2f5

assign unique IDs to comment forms publiclab#9004

f2f219b

noi5e requested a review from a team as a code owner January 21, 2021 23:00

noi5e added JavaScript outreachy feature explains that the issue is to add a new feature labels Jan 21, 2021

noi5e added this to the [Outreachy] Comment Editor Winter 2020 milestone Jan 21, 2021

change selectors from #c1edit to #comment-form-edit-1 publiclab#9004

47610e1

noi5e requested review from cesswairimu, jywarren and sagarpreet-chadha January 21, 2021 23:46

noi5e commented Jan 21, 2021

View reviewed changes

noi5e added the readytomerge label Jan 21, 2021

noi5e mentioned this pull request Jan 21, 2021

OLD CLOSED Comment Editor Overhaul Issue #8775

Closed

noi5e added 2 commits January 21, 2021 16:18

change preview IDs from #preview1 to #comment-preview-1 publiclab#9004

66ddada

change main preview selector to #comment-preview-main from #preview-m…

fcfaa23

…ain publiclab#9004

noi5e requested a review from a team as a code owner January 22, 2021 00:46

noi5e commented Jan 22, 2021

View reviewed changes

change main preview selector to #comment-preview-main from #preview-m…

698c8cf

…ain publiclab#9004

This was referenced Jan 23, 2021

Integrate Edit Comment Form JavaScript with editor.js #9067

Merged

Comment Editor Overhaul Project [Outreachy] #9069

Open

jywarren approved these changes Jan 26, 2021

View reviewed changes

jywarren merged commit 7b4bc4d into publiclab:main Jan 26, 2021

noi5e deleted the unique-comment-form-IDs branch January 26, 2021 20:37

noi5e mentioned this pull request Feb 2, 2021

Refactor Editor Constructor and Methods, Write Editor Getters #9110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Unique IDs to Comment Forms #9062

Add Unique IDs to Comment Forms #9062

noi5e commented Jan 21, 2021 •

edited

Loading

gitpod-io bot commented Jan 21, 2021 •

edited

Loading

codecov bot commented Jan 21, 2021 •

edited

Loading

noi5e Jan 21, 2021

noi5e Jan 21, 2021

noi5e Jan 21, 2021

noi5e Jan 22, 2021

codeclimate bot commented Jan 22, 2021

noi5e commented Jan 22, 2021 •

edited

Loading

jywarren commented Jan 26, 2021

jywarren left a comment

noi5e commented Jan 26, 2021

jywarren commented Jan 26, 2021

Add Unique IDs to Comment Forms #9062

Add Unique IDs to Comment Forms #9062

Conversation

noi5e commented Jan 21, 2021 • edited Loading

gitpod-io bot commented Jan 21, 2021 • edited Loading

codecov bot commented Jan 21, 2021 • edited Loading

Codecov Report

noi5e Jan 21, 2021

Choose a reason for hiding this comment

noi5e Jan 21, 2021

Choose a reason for hiding this comment

noi5e Jan 21, 2021

Choose a reason for hiding this comment

noi5e Jan 22, 2021

Choose a reason for hiding this comment

codeclimate bot commented Jan 22, 2021

noi5e commented Jan 22, 2021 • edited Loading

jywarren commented Jan 26, 2021

jywarren left a comment

Choose a reason for hiding this comment

noi5e commented Jan 26, 2021

jywarren commented Jan 26, 2021

noi5e commented Jan 21, 2021 •

edited

Loading

gitpod-io bot commented Jan 21, 2021 •

edited

Loading

codecov bot commented Jan 21, 2021 •

edited

Loading

noi5e commented Jan 22, 2021 •

edited

Loading