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.

Sign up for GitHub

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

Jump to bottom

Prune Editor State Management #9108

Merged
merged 12 commits into from
Feb 4, 2021
Merged

Prune Editor State Management #9108

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
db68524
merge editor refactor branches #9004
noi5e Feb 1, 2021
5419373
delete refresh & isSingleFormPage methods #9004
noi5e Feb 1, 2021
71c933c
delete some D.selected assignments #9004
noi5e Feb 1, 2021
c91a92d
add dropzone class to choose one input elements
noi5e Feb 1, 2021
afb8328
E.setState on image select upload #9004
noi5e Feb 1, 2021
9269c12
remove D.selected from progress bar handlers #9004
noi5e Feb 1, 2021
daa4db5
add unique IDs to image upload progress bars #9004
noi5e Feb 1, 2021
f9bee00
updated image upload progress bar selectors #9004
noi5e Feb 1, 2021
810646c
update progress bar selector #9004
noi5e Feb 1, 2021
601240c
update selectors for image upload text #9004
noi5e Feb 1, 2021
460cf12
Merge branch 'main' into prune-editor-state-management
noi5e Feb 4, 2021
919208e
Merge branch 'main' into prune-editor-state-management
noi5e Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 14 additions & 38 deletions app/assets/javascripts/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
class Editor {
// default parameters reference the IDs of:
// 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(textarea = "text-input", preview = "comment-preview-main", title = "title") {
this.textarea = $('#' + textarea);
this.preview = $('#' + preview);
this.title = $('#' + title + 'title'); // not sure why this exists? seems like $E.title is always #title
constructor(commentFormID = "main") {
this.commentFormID = commentFormID;
this.textarea = $("#text-input-" + commentFormID);
this.preview = $("#comment-preview-" + commentFormID);
this.previewing = false;
this.previewed = false;
// this will get deleted in the next few PRs, so collapsing into one line to pass codeclimate
Expand All @@ -27,36 +27,14 @@ class Editor {
}
});
}
setState(textarea = 'text-input', preview = 'comment-preview-main', title = 'title') {
$E.title = $('#' + title + 'title'); // not sure why this exists? seems like $E.title is always #title
$E.textarea = $('#' + textarea);
setState(commentFormID = "main") {
this.commentFormID = commentFormID;
$E.textarea = $("#text-input-" + commentFormID);
$E.textarea.bind('input propertychange', $E.save);
$E.preview = $('#' + preview);
}
// code seems unused, commenting out for now.
// is_editing() {
// return ($E.textarea[0].selectionStart == 0 && $E.textarea[0].selectionEnd == 0)
// };
refresh() {
// textarea
$E.textarea = ($D.selected).find('textarea').eq(0);
$E.textarea.bind('input propertychange',$E.save);
// preview
$E.preview = ($D.selected).find('.comment-preview').eq(0);
}
isSingleFormPage(url) {
// this RegEx matches three different pages where only one editor form is present (instead of multiple comment forms):
// 1. /wiki/new
// 2. /wiki/{wiki name}/edit
// 3. /features/new
const singleFormPath = RegExp(/\/(wiki|features)(\/[^\/]+\/edit|\/new)/);
return singleFormPath.test(url);
$E.preview = $("#comment-preview-" + commentFormID);
}
// wraps currently selected text in textarea with strings a and b
wrap(a, b, newlineDesired = false, fallback) {
// we only refresh $E's values if we are on a page with multiple comments
if (!this.isSingleFormPage(window.location.pathname)) { this.refresh(); }

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
Expand Down Expand Up @@ -109,12 +87,10 @@ class Editor {
// }
// this function is dedicated to Don Blair https://github.com/donblair
save() {
localStorage.setItem('plots:lastpost',$E.textarea.val())
localStorage.setItem('plots:lasttitle',$E.title.val())
localStorage.setItem('plots:lastpost',$E.textarea.val());
}
recover() {
$E.textarea.val(localStorage.getItem('plots:lastpost'))
$E.title.val(localStorage.getItem('plots:lasttitle'))
$E.textarea.val(localStorage.getItem('plots:lastpost'));
}
apply_template(template) {
if($E.textarea.val() == ""){
Expand All @@ -133,8 +109,8 @@ class Editor {
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 = $(this.textarea.context).find('.preview-btn');
dropzone = $(this.textarea.context).find('.dropzone');
previewBtn = $("#toggle-preview-button-" + this.commentFormID);
dropzone = $("#dropzone-large-" + this.commentFormID);

$E.preview[0].innerHTML = marked($E.textarea.val());
$E.preview.toggle();
Expand All @@ -161,4 +137,4 @@ class Editor {
previewBtn.text('Preview');
}
}
}
}
59 changes: 14 additions & 45 deletions app/assets/javascripts/editorToolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,9 @@
// this script is used in a variety of different contexts including:
// this script is used wherever the legacy editor is used.
// pages (wikis, questions, research notes) with multiple comments & editors for each comment
// pages with JUST ONE form, and no other comments, eg. /wiki/new & /wiki/edit
// /app/views/features/_form.html.erb
// /app/views/map/edit.html.erb
// the legacy editor: /app/views/editor/_editor.html.erb (if it's still in use live?)

const getEditorParams = (targetDiv) => {
const closestCommentFormWrapper = targetDiv.closest('div.comment-form-wrapper'); // this returns null if there is no match
Copy link
Contributor

Choose a reason for hiding this comment

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

This legacy code is not at all good, using ids everywhere is so awesome 💯

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 know right!!!

let params = {};
// there are no .comment-form-wrappers on /wiki/edit or /wiki/new
// these pages just have a single text-input form.
if (closestCommentFormWrapper) {
params['dSelected'] = $(closestCommentFormWrapper);
// assign the ID of the textarea within the closest comment-form-wrapper
params['textarea'] = closestCommentFormWrapper.querySelector('textarea').id;
params['preview'] = closestCommentFormWrapper.querySelector('.comment-preview').id;
} else {
// default to #text-input
// #text-input ID should be unique, and the only comment form on /wiki/new & /wiki/edit
params['textarea'] = 'text-input';
// #preview-main should be unique as well
params['preview'] = 'comment-preview-main';
}
return params;
};
// and wherever /app/views/editor/editor.html.erb is still used in production

const progressAll = (elem, data) => {
var progress = parseInt(data.loaded / data.total * 100, 10);
Expand All @@ -36,13 +16,8 @@ const progressAll = (elem, data) => {
// attach eventListeners on document.load for toolbar rich-text buttons & image upload .dropzones
$(function() {
// for rich-text buttons (bold, italic, header, and link):
// click eventHandler that assigns $D.selected to the appropriate comment form
// on pages with multiple comments, $D.selected needs to be accurate so that rich-text changes (bold, italic, etc.) go into the right comment form
$('.rich-text-button').on('click', function(e) {
const { textArea, preview, dSelected } = getEditorParams(e.target);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

completely removed this getEditorParams helper function and everywhere it's called!

// assign dSelected
if (dSelected) { $D.selected = dSelected; }
$E.setState(textArea, preview);
$E.setState(e.currentTarget.dataset.formId); // string that is: "main", "reply-123", "edit-123" etc.
const action = e.currentTarget.dataset.action // 'bold', 'italic', etc.
$E[action](); // call the appropriate editor function
});
Expand All @@ -65,10 +40,8 @@ $(function() {

// runs on drag & drop
$(this).on('drop',function(e) {
const { textArea, preview, dSelected } = getEditorParams(e.target);
e.preventDefault();
if (dSelected) { $D.selected = dSelected; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$D.selected is now gone too!

$E.setState(textArea, preview);
$E.setState(e.currentTarget.dataset.formId); // string that is: "main", "reply-123", "edit-123" etc.
});

$(this).fileupload({
Expand All @@ -84,22 +57,18 @@ $(function() {
// 1. when user drag-and-drops image
// 2. when user clicks on upload button.
start: function(e) {
$E.setState(e.target.dataset.formId); // string that is: "main", "reply-123", "edit-123" etc.
$(e.target).removeClass('hover');
// for click-upload-button scenarios, it's important to set $D.selected here, because the 'drop' listener above doesn't run in those:
$D.selected = $(e.target).closest('div.comment-form-wrapper');
// the above line is redundant in drag & drop, because it's assigned in 'drop' listener too.
// on /wiki/new & /wiki/edit, $D.selected will = undefined from this assignment
elem = $($D.selected).closest('div.comment-form-wrapper').eq(0);
elem.find('.progress-bar-container').eq(0).show();
elem.find('.uploading-text').eq(0).show();
elem.find('.choose-one-prompt-text').eq(0).hide();
console.log($("#image-upload-progress-container-" + $E.commentFormID));
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, yeah, I didn't catch this one! I think I remove this in a later PR, but just in case, adding it to my list.

$("#image-upload-progress-container-" + $E.commentFormID).show();
$("#image-upload-text-" + $E.commentFormID).show();
$("#dropzone-choose-one-" + $E.commentFormID).hide();
},
done: function (e, data) {
elem = $($D.selected).closest('div.comment-form-wrapper').eq(0);
elem.find('.progress-bar-container').hide();
elem.find('.progress-bar').css('width', 0);
elem.find('.uploading-text').hide();
elem.find('.choose-one-prompt-text').show();
$("#image-upload-progress-container-" + $E.commentFormID).hide();
$("#image-upload-text-" + $E.commentFormID).hide();
$("#dropzone-choose-one-" + $E.commentFormID).show();
$("#image-upload-progress-bar-" + $E.commentFormID).css('width', 0);
var extension = data.result['filename'].split('.')[data.result['filename'].split('.').length - 1]; var file_url = data.result.url.split('?')[0]; var file_type;
if (['gif', 'GIF', 'jpeg', 'JPEG', 'jpg', 'JPG', 'png', 'PNG'].includes(extension))
file_type = 'image'
Expand All @@ -124,7 +93,7 @@ $(function() {
console.log(e);
},
progressall: function (e, data) {
const closestProgressBar = $($D.selected).closest('div.comment-form-wrapper').find('.progress-bar').eq(0);
const closestProgressBar = $("#image-upload-progress-bar-" + $E.commentFormID);
return progressAll(closestProgressBar, data);
}
});
Expand Down
14 changes: 1 addition & 13 deletions app/helpers/application_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,7 @@ def feature_node(title)
end

# used in views/comments/_form.html.erb
def get_comment_form_id(location, reply_to)
case location
when :main
'main'
when :reply
'reply-' + reply_to.to_s
when :responses
'responses'
end
end

# used in views/editor/_toolbar.html.erb
def get_toolbar_element_id(location, reply_to, comment_id)
def get_comment_form_id(location, reply_to, comment_id)
case location
when :main
'main'
Expand Down
Loading