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

Prune Editor State Management #9108

Merged
merged 12 commits into from
Feb 4, 2021
22 changes: 0 additions & 22 deletions app/assets/javascripts/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,8 @@ class Editor {
$E.textarea.bind('input propertychange', $E.save);
$E.preview = $("#comment-preview-" + commentFormID);
}
// 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);
}
// 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
59 changes: 14 additions & 45 deletions app/assets/javascripts/editorToolbar.js
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-main
// #text-input-main ID should be unique, and the only comment form on /wiki/new & /wiki/edit
params['textarea'] = 'text-input-main';
// #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(e.currentTarget.dataset.formId);
$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(e.currentTarget.dataset.formId);
$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
41 changes: 24 additions & 17 deletions app/views/comments/_edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<h4 style="margin-top:0;"><%= title %></h4>
<input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>">
<style>
#imagebar {width:100%;}
.imagebar {width:100%;}
</style>
<!-- toolbar needs location & comment_id to make unique element IDs -->
<%= render :partial => "editor/toolbar", :locals => { :comment_id => comment.id.to_s, :location => :edit } %>
Expand All @@ -29,30 +29,37 @@
placeholder="<%= placeholder %>"
><%= !(comment.is_a?Answer) ? comment.comment : comment.content %></textarea>
<div class="imagebar">
<div id="c<%= comment.id%>progress" style="display:none;" class="progress progress-bar-container active pull-right">
<div id="c<%= comment.id%>progress-bar" class="progress-bar progress-bar-striped progress-bar-animated" style="width: 0%;"></div>
<div
id="image-upload-progress-container-edit-<%= comment.id %>" style="display: none;"
class="progress progress-bar-container active pull-right"
>
<div
id="image-upload-progress-bar-edit-<%= comment.id %>"
class="progress-bar progress-bar-striped progress-bar-animated" style="width: 0%;"
>
</div>
</div>
<p>
<span id="c<%= comment.id%>uploading" class="uploading uploading-text" style="display:none;">
<span
id="image-upload-text-edit-<%= comment.id %>"
class="uploading uploading-text"
style="display:none;"
>
<%= translation('comments._edit.uploading') %>
</span>
<span id="c<%= comment.id%>prompt" class="prompt choose-one-prompt-text">
<span id="dropzone-choose-one-edit-<%= comment.id%>" class="prompt choose-one-prompt-text dropzone">
<span style="padding-right:4px;float:left;" class="hidden-xs">
<%= raw translation('comments._edit.drag_and_drop') %>
</span>
<span id="c<%= comment.id%>prompt" class="prompt choose-one-prompt-text">
<span style="padding-right:4px;float:left;" class="hidden-xs">
<%= raw translation('comments._edit.drag_and_drop') %>
<label id="c<%= comment.id%>input_label" for="fileinput-choose-one-edit-<%= comment.id%>">
<input id="fileinput-choose-one-edit-<%= comment.id%>" type="file" name="image[photo]" style="display:none;" />
<a class="hidden-xs"><%= translation('comments._edit.choose_one') %></a>
<span class="visible-xs">
<i class="fa fa-upload"></i>
<a><%= translation('comments._edit.upload_image') %></a>
</span>
<label id="c<%= comment.id%>input_label" for="c<%= comment.id%>input">
<input id="c<%= comment.id%>input" type="file" name="image[photo]" style="display:none;" />
<a class="hidden-xs"><%= translation('comments._edit.choose_one') %></a>
<span class="visible-xs">
<i class="fa fa-upload"></i>
<a><%= translation('comments._edit.upload_image') %></a>
</span>
</label>
</span>
</label>
</span>
</p>
</div>
</div>
Expand Down
20 changes: 16 additions & 4 deletions app/views/comments/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,26 @@
placeholder="<%= placeholder %>"><%= body %>
</textarea>
<div class="imagebar">
<div style="display:none;" class="progress progress-bar-container float-right">
<div class="progress-bar progress-bar-striped progress-bar-animated" style="width: 0%;"></div>
<div
id="image-upload-progress-container-<%= comment_form_id %>"
style="display:none;"
class="progress progress-bar-container float-right"
>
<div
id="image-upload-progress-bar-<%= comment_form_id %>"
class="progress-bar progress-bar-striped progress-bar-animated"
style="width: 0%;">
</div>
</div>
<p>
<span class="uploading-text" style="display:none;">
<span
id="image-upload-text-<%= comment_form_id %>"
class="uploading-text"
style="display:none;"
>
<%= translation('comments._form.uploading') %>
</span>
<span class="prompt choose-one-prompt-text">
<span id="dropzone-choose-one-<%= comment_form_id %>" class="prompt choose-one-prompt-text dropzone">
<span style="padding-right:4px;float:left;" class="d-none d-md-inline">
<%= raw translation('comments._form.drag_and_drop') %>
</span>
Expand Down
4 changes: 2 additions & 2 deletions test/system/comment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ def get_path(page_type, path)
# find edit comment's fileinput:
page.find('#fileinput-button-edit-' + comment_id_num).set("#{Rails.root.to_s}/public/images/pl.png")
Capybara.ignore_hidden_elements = true
assert_selector('#c' + comment_id_num + 'progress')
assert_selector('#c' + comment_id_num + 'uploading')
assert_selector('#image-upload-progress-container-edit-' + comment_id_num)
assert_selector('#image-upload-text-edit-' + comment_id_num)
end
end

Expand Down