Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Editor Constructor and Methods, Write Editor Getters #9110

Merged
merged 11 commits into from
Feb 4, 2021
113 changes: 67 additions & 46 deletions app/assets/javascripts/editor.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
class Editor {
// default parameter references the ID of:
// 1. main comment form in multi-comment wikis, questions, & research notes.
// 2. the only editor form on /wiki/new and /wiki/edit
constructor(commentFormID = "main") {
this.commentFormID = commentFormID;
this.textarea = $("#text-input-" + commentFormID);
this.preview = $("#comment-preview-" + commentFormID);
this.previewing = false;
this.previewed = false;
// default parameters:
// defaultForm - when the Editor is initialized, there needs to be a default editor form:
// 1. the main comment form in multi-comment wikis, questions, & research notes.
// 2. the only editor form on /wiki/new and /wiki/edit
// elementIDPrefixes - the ID naming conventions are different depending on context:
// 1. multi-form pages with multiple comments: #comment-preview-123
// 2. /wiki/new and /wiki/edit: #preview-main
constructor(defaultForm = "main", elementIDPrefixes = {}) {
this.commentFormID = defaultForm;
this.elementIDPrefixes = elementIDPrefixes;
this.textarea = $("#text-input-" + this.commentFormID);
// this will get deleted in the next few PRs, so collapsing into one line to pass codeclimate
this.templates = { 'blog': "## The beginning\n\n## What we did\n\n## Why it matters\n\n## How can you help", 'default': "## What I want to do\n\n## My attempt and results\n\n## Questions and next steps\n\n## Why I'm interested", 'support': "## Details about the problem\n\n## A photo or screenshot of the setup", 'event': "## Event details\n\nWhen, where, what\n\n## Background\n\nWho, why", 'question': "## What I want to do or know\n\n## Background story" };

Expand All @@ -27,93 +29,112 @@ class Editor {
}
});
}
setState(commentFormID = "main") {
setState(commentFormID) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the default parameter (commentFormID = "main") here. I think it makes sense so that errors will be easier to catch.

this.commentFormID = commentFormID;
$E.textarea = $("#text-input-" + commentFormID);
this.textarea = $("#text-input-" + commentFormID);
$E.textarea.bind('input propertychange', $E.save);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed most instances of $E.method to this.method except for this one here. I toyed around with this, but it opens up a can of worms and is out of scope of this PR. Will go back to this when I refactor save and recover functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to explain this a little further, it's because when $E.textarea is changed to this.textarea it brings up JavaScript this weirdness. The eventListener for input propertychange (which I'm not sure is a valid event type? I tried Googling it) gets attached to the wrong this.

$E.preview = $("#comment-preview-" + commentFormID);
}
get textAreaElement() {
const textAreaID = "#text-input-" + this.commentFormID;
return $(textAreaID);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New getter functions!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oohhhhh!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are these transpiled out in the asset pipeline? Any worries about internet explorer? I guess not these days!

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get

I mean, i guess it's still 5% of usage: https://www.netmarketshare.com/browser-market-share.aspx?qprid=2&qpcustomd=0

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh huh... That's a good point. I hadn't checked this one out in MDN.

I think they can be rewritten as regular methods! That won't be too hard. Can do it in another PR although... Do our users really use IE to access the website? Better safe than sorry I guess.

get textAreaValue() {
return this.textAreaElement.val();
}
get previewElement() {
let previewIDPrefix = "#comment-preview-";
if (this.elementIDPrefixes.hasOwnProperty("preview")) {
// eg. on /wiki/new & /wiki/edit, the preview element is called #preview-main
previewIDPrefix = "#" + this.elementIDPrefixes["preview"] + "-";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the main changes in this PR. Honestly, I have mixed feelings about it since it's a little weird, especially in the constructor.

But I do think it's an okay workaround. I made this because there was some tension in having editor.js operative on both 1) multi-comment pages and 2) /wiki/new and /wiki/edit.

I wanted preview elements to be named #comment-preview-reply-123 (they used to just be called #preview), for greater transparency. But that clashes with the wiki editor, where there are no comments involved.

So, this is my workaround. It's a little weird, but thanks to OOP, it can be scaled back or revised much easier.

}
const previewID = previewIDPrefix + this.commentFormID;
return $(previewID);
}
// wraps currently selected text in textarea with strings a and b
wrap(a, b, newlineDesired = false, fallback) {
const selectionStart = $E.textarea[0].selectionStart;
const selectionEnd = $E.textarea[0].selectionEnd;
const selection = fallback || $E.textarea.val().substring(selectionStart, selectionEnd); // fallback if nothing has been selected, and we're simply dealing with an insertion point
const selectionStart = this.textAreaElement[0].selectionStart;
const selectionEnd = this.textAreaElement[0].selectionEnd;
const selection = fallback || this.textAreaValue.substring(selectionStart, selectionEnd); // fallback if nothing has been selected, and we're simply dealing with an insertion point

let newText = newlineDesired ? a + selection + b + "\n\n" : a + selection + b; // ie. ** + selection + ** (wrapping selection in bold)
const selectionStartsMidText = $E.textarea[0].selectionStart > 0;
const selectionStartsMidText = this.textAreaElement[0].selectionStart > 0;
if (newlineDesired && selectionStartsMidText) { newText = "\n" + newText; }

const textLength = $E.textarea.val().length;
const textBeforeSelection = $E.textarea.val().substring(0, selectionStart);
const textAfterSelection = $E.textarea.val().substring(selectionEnd, textLength);
$E.textarea.val(textBeforeSelection + newText + textAfterSelection);
const textLength = this.textAreaValue.length;
const textBeforeSelection = this.textAreaValue.substring(0, selectionStart);
const textAfterSelection = this.textAreaValue.substring(selectionEnd, textLength);
this.textAreaElement.val(textBeforeSelection + newText + textAfterSelection);
}
bold() {
$E.wrap('**','**')
this.wrap('**', '**');
}
italic() {
$E.wrap('_','_')
this.wrap('_', '_');
}
link(uri) {
uri = prompt('Enter a URL');
if (!uri) { uri = ""; }
$E.wrap('[', '](' + uri + ')');
if (uri === null) { uri = ""; }
noi5e marked this conversation as resolved.
Show resolved Hide resolved
this.wrap(
'[',
'](' + uri + ')'
);
}
image(src) {
$E.wrap('\n![',']('+src+')\n')
this.wrap(
'\n![',
'](' + src + ')\n'
);
}
// these header formatting functions are not used anywhere, so commenting them out for now to pass codeclimate:

// h1() {
// $E.wrap('#','')
// this.wrap('#','')
// }
h2() {
$E.wrap('##','')
this.wrap('##', '');
}
// h3() {
// $E.wrap('###','')
// this.wrap('###','')
// }
// h4() {
// $E.wrap('####','')
// this.wrap('####','')
// }
// h5() {
// $E.wrap('#####','')
// this.wrap('#####','')
// }
// h6() {
// $E.wrap('######','')
// this.wrap('######','')
// }
// h7() {
// $E.wrap('#######','')
// this.wrap('#######','')
// }
// this function is dedicated to Don Blair https://github.com/donblair
save() {
localStorage.setItem('plots:lastpost',$E.textarea.val());
localStorage.setItem('plots:lastpost', this.textAreaValue);
}
recover() {
$E.textarea.val(localStorage.getItem('plots:lastpost'));
this.textAreaElement.val(localStorage.getItem('plots:lastpost'));
}
apply_template(template) {
if($E.textarea.val() == ""){
$E.textarea.val($E.templates[template])
}else if(($E.textarea.val() == $E.templates['event']) || ($E.textarea.val() == $E.templates['default']) || ($E.textarea.val() == $E.templates['support'])){
$E.textarea.val($E.templates[template])
if(this.textAreaValue == ""){
this.textAreaElement.val(this.templates[template])
}else if((this.textAreaValue == this.templates['event']) || (this.textAreaValue == this.templates['default']) || (this.textAreaValue == this.templates['support'])){
this.textAreaElement.val(this.templates[template])
}else{
$E.textarea.val($E.textarea.val()+'\n\n'+$E.templates[template])
this.textAreaElement.val(this.textAreaValue+'\n\n'+this.templates[template])
}
}
generate_preview(id,text) {
$('#'+id)[0].innerHTML = marked(text)
$('#' + id)[0].innerHTML = marked(text)
}
toggle_preview() {
let previewBtn;
let dropzone;
// if the element is part of a multi-comment page,
// ensure to grab the current element and not the other comment element.
previewBtn = $("#toggle-preview-button-" + this.commentFormID);
dropzone = $("#dropzone-large-" + this.commentFormID);
const previewBtn = $("#toggle-preview-button-" + this.commentFormID);
const dropzone = $("#dropzone-large-" + this.commentFormID);

$E.preview[0].innerHTML = marked($E.textarea.val());
$E.preview.toggle();
this.previewElement[0].innerHTML = marked(this.textAreaValue);
this.previewElement.toggle();
dropzone.toggle();

this.toggleButtonPreviewMode(previewBtn);
Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jQuery(document).ready(function() {
if (e.which == 32 || e.which == 13) publish()
})

if ($E.textarea.val() == "") $E.apply_template(getUrlParameter('template') || "default")
if ($E.textAreaValue === "") $E.apply_template(getUrlParameter('template') || "default")
noi5e marked this conversation as resolved.
Show resolved Hide resolved

/* tag autocomplete */
$('#taginput').typeahead({
Expand Down
2 changes: 1 addition & 1 deletion app/views/comments/_edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
>
<%= translation('comments._edit.uploading') %>
</span>
<span id="dropzone-choose-one-edit-<%= comment.id%>" class="prompt choose-one-prompt-text dropzone">
<span id="dropzone-choose-one-edit-<%= comment.id%>" class="prompt choose-one-prompt-text">
<span style="padding-right:4px;float:left;" class="hidden-xs">
<%= raw translation('comments._edit.drag_and_drop') %>
</span>
Expand Down
2 changes: 1 addition & 1 deletion app/views/comments/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
>
<%= translation('comments._form.uploading') %>
</span>
<span id="dropzone-choose-one-<%= comment_form_id %>" class="prompt choose-one-prompt-text dropzone">
<span id="dropzone-choose-one-<%= comment_form_id %>" class="prompt choose-one-prompt-text">
<span style="padding-right:4px;float:left;" class="d-none d-md-inline">
<%= raw translation('comments._form.drag_and_drop') %>
</span>
Expand Down
34 changes: 23 additions & 11 deletions app/views/editor/_editor.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,36 @@

<div class="form-group dropzone">
<textarea aria-label="Wiki Text" name="body" tabindex="2" class="form-control" id="text-input-main" rows="20" cols="60"><% if @node && @node.latest && @node.latest.body %><%= @node.latest.body %><% else %><%= params[:body] %><% end %></textarea>
<div id="imagebar">
<div class = "inline_image_drop" style="display:none;">
<div id="create_progress" style="display:none;" class="progress float-right">
<div id="create_progress-bar" class="progress-bar progress-bar-striped progress-bar-animated" style="width: 0%;"></div>
<div class="imagebar">
<div
id="image-upload-progress-container-main" style="display:none;"
class="progress progress-bar-container float-right"
>
<div
id="image-upload-progress-bar-main"
class="progress-bar progress-bar-striped progress-bar-animated"
style="width: 0%;"
>
</div>
<p>
<span id="create_uploading" class="uploading" style="display:none;">
Uploading...
</span>
</p>
</div>
<p>
<span
id="image-upload-text-main"
class="uploading-text"
style="display:none;">
Uploading...
</span>
</p>
</div>
</div>

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

<script>
jQuery(document).ready(function() {
$E = new Editor();
$E = new Editor(
"main",
{ "preview": "preview" } // the preview element here is #preview-main, not #comment-preview-main
);
})
</script>
2 changes: 1 addition & 1 deletion test/system/post_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def assert_page_does_not_reload(message = "page should not reload")
find('.preview-btn').click()

# Make sure that image has been uploaded
page.assert_selector('#comment-preview-main img', count: 1)
page.assert_selector('#preview-main img', count: 1)
end

test "changing and reverting versions works correctly for wiki" do
Expand Down