Add setState Method in editor.js #9035

noi5e · 2021-01-18T23:32:47Z

This is a small part of a larger object-oriented refactoring for editor.js, see #9004 for details.

Basically the major change here is that I am creating a setState method for use in cases like this:

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

Lines 42 to 48 in f0be879

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

I think $E.initialize is used for two different cases, and I'm trying to make a greater distinction between the two:

$E.initialize when the document loads, and the editor must point toward the main comment form (or the only editor form on the page, in the case of /wiki/new, /wiki/edit, and /features/new
$E.initialize when we want the editor textarea to change on a multi-comment page, as in the above example. For this case, we can use the new setState method instead.

I have confidence this is going to help!!!

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

…b#9004

gitpod-io · 2021-01-18T23:32:51Z

codecov · 2021-01-18T23:38:36Z

Codecov Report

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

@@           Coverage Diff           @@
##             main    #9035   +/-   ##
=======================================
  Coverage        ?   82.05%           
=======================================
  Files           ?      100           
  Lines           ?     5939           
  Branches        ?        0           
=======================================
  Hits            ?     4873           
  Misses          ?     1066           
  Partials        ?        0

noi5e · 2021-01-18T23:55:21Z

app/assets/javascripts/dragdrop.js

@@ -29,12 +29,10 @@ jQuery(function() {
    });

    $(this).on('drop',function(e) {
-      const params = getEditorParams(e.target); // defined in editorHelper.js
+      const { textArea, preview, dSelected } = getEditorParams(e.target); // defined in editorHelper.js


aka ES6 destructuring assignment

Oohhhh!!! 🤗

jywarren

Looks great! I like the separation of functions!

app/assets/javascripts/editor.js

noi5e · 2021-01-19T04:43:59Z

app/assets/javascripts/editor.js

+  wrap: function(a, b, args) {
+    // we only refresh $E's values if we are on a page using the rich-text editor (most pages).
+    // the legacy editor pages only have one editor form, unlike pages with multiple comments.
+    if (this.isRichTextEditor(window.location.pathname)) { this.refresh(); }
    var len = $E.textarea.val().length;
    var start = $E.textarea[0].selectionStart;
    var end = $E.textarea[0].selectionEnd;


@jywarren codeclimate won't let this pass because of 'cognitive complexity' in the wrap function... I think the rules are stricter now since the recent changes in CI.

Honestly, I have a feeling that it's the older stuff later on in the method that's holding this back (conditionals like args && args['fallback'], a nested if statement inside if (args && args['newline']). I'm not very familiar with this code yet, can you take a look and let me know if you have any ideas to simplify things here so this can pass?

@jywarren Never mind! I got it to work. Tests are passing now too.

Ready to merge!

codeclimate · 2021-01-19T05:28:13Z

Code Climate has analyzed commit 2f04589 and detected 0 issues on this pull request.

View more on Code Climate.

cesswairimu

Wow 🚀 Great job

jywarren · 2021-01-19T12:50:33Z

Awesome work!!!!!

…

On Tue, Jan 19, 2021, 4:37 AM Cess ***@***.***> wrote: Merged #9035 <#9035> into main. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9035 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J7FZMDLRSTBUHDDCRTS2VHGTANCNFSM4WH6VBPA> .

* write setState method with default parameters publiclab#9004 * set preview when calling E.setState publiclab#9004 * set preview when calling E.setState publiclab#9004 * change E.initialize to E.setState; add comment-preview class publiclab#9004 * change E.initialize({}) to E.initialize() publiclab#9004 * change regex test to match /features/new in wrap publiclab#9004 * rewrite selector for more specificity publiclab#9004 * rewrite for cognitive complexity publiclab#9004 * desperately trying to pass codeclimate publiclab#9004 * desperately trying to pass codeclimate publiclab#9004 * desperately trying to pass codeclimate publiclab#9004 * desperately trying to pass codeclimate publiclab#9004 * oops, switched ternary operator publiclab#9004

noi5e added 5 commits January 18, 2021 15:26

write setState method with default parameters publiclab#9004

298355f

set preview when calling E.setState publiclab#9004

dda4d2d

set preview when calling E.setState publiclab#9004

a4ae7c3

change E.initialize to E.setState; add comment-preview class publicla…

aff84c9

…b#9004

change E.initialize({}) to E.initialize() publiclab#9004

7a64190

noi5e requested review from a team as code owners January 18, 2021 23:32

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

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

noi5e commented Jan 18, 2021

View reviewed changes

noi5e added 3 commits January 18, 2021 16:17

change regex test to match /features/new in wrap publiclab#9004

e0b148b

rewrite selector for more specificity publiclab#9004

2af718b

rewrite for cognitive complexity publiclab#9004

0784378

jywarren approved these changes Jan 19, 2021

View reviewed changes

desperately trying to pass codeclimate publiclab#9004

aacb494

codeclimate bot reviewed Jan 19, 2021

View reviewed changes

app/assets/javascripts/editor.js Outdated Show resolved Hide resolved

desperately trying to pass codeclimate publiclab#9004

16447e2

codeclimate bot reviewed Jan 19, 2021

View reviewed changes

app/assets/javascripts/editor.js Show resolved Hide resolved

noi5e commented Jan 19, 2021

View reviewed changes

noi5e added 3 commits January 18, 2021 20:49

desperately trying to pass codeclimate publiclab#9004

ca93f14

desperately trying to pass codeclimate publiclab#9004

f44a6bb

oops, switched ternary operator publiclab#9004

2f04589

noi5e requested a review from jywarren January 19, 2021 05:51

noi5e added the readytomerge label Jan 19, 2021

noi5e mentioned this pull request Jan 19, 2021

OLD CLOSED Comment Editor Overhaul Issue #8775

Closed

cesswairimu approved these changes Jan 19, 2021

View reviewed changes

cesswairimu merged commit f82a11c into publiclab:main Jan 19, 2021

noi5e deleted the feature/E-initialize-default-params branch January 19, 2021 17:01

noi5e mentioned this pull request Jan 23, 2021

Comment Editor Overhaul Project [Outreachy] #9069

Open

noi5e changed the title ~~Feature: Add setState Method in editor.js~~ Add setState Method in editor.js Jan 23, 2021

noi5e mentioned this pull request Jan 31, 2021

Refactor editor.js with Object-Oriented Principles #9104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add setState Method in editor.js #9035

Add setState Method in editor.js #9035

noi5e commented Jan 18, 2021 •

edited

Loading

gitpod-io bot commented Jan 18, 2021 •

edited

Loading

codecov bot commented Jan 18, 2021 •

edited

Loading

noi5e Jan 18, 2021

jywarren Jan 19, 2021

jywarren left a comment

noi5e Jan 19, 2021

noi5e Jan 19, 2021

codeclimate bot commented Jan 19, 2021

cesswairimu left a comment

jywarren commented Jan 19, 2021 via email

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

Add setState Method in editor.js #9035

Add setState Method in editor.js #9035

Conversation

noi5e commented Jan 18, 2021 • edited Loading

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

codecov bot commented Jan 18, 2021 • edited Loading

Codecov Report

noi5e Jan 18, 2021

Choose a reason for hiding this comment

jywarren Jan 19, 2021

Choose a reason for hiding this comment

jywarren left a comment

Choose a reason for hiding this comment

noi5e Jan 19, 2021

Choose a reason for hiding this comment

noi5e Jan 19, 2021

Choose a reason for hiding this comment

codeclimate bot commented Jan 19, 2021

cesswairimu left a comment

Choose a reason for hiding this comment

jywarren commented Jan 19, 2021 via email

noi5e commented Jan 18, 2021 •

edited

Loading

gitpod-io bot commented Jan 18, 2021 •

edited

Loading

codecov bot commented Jan 18, 2021 •

edited

Loading