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

Add setState Method in editor.js #9035

Merged
merged 13 commits into from
Jan 19, 2021
Merged

Add setState Method in editor.js #9035

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
298355f
write setState method with default parameters #9004
noi5e Jan 18, 2021
dda4d2d
set preview when calling E.setState #9004
noi5e Jan 18, 2021
a4ae7c3
set preview when calling E.setState #9004
noi5e Jan 18, 2021
aff84c9
change E.initialize to E.setState; add comment-preview class #9004
noi5e Jan 18, 2021
7a64190
change E.initialize({}) to E.initialize() #9004
noi5e Jan 18, 2021
e0b148b
change regex test to match /features/new in wrap #9004
noi5e Jan 19, 2021
2af718b
rewrite selector for more specificity #9004
noi5e Jan 19, 2021
0784378
rewrite for cognitive complexity #9004
noi5e Jan 19, 2021
aacb494
desperately trying to pass codeclimate #9004
noi5e Jan 19, 2021
16447e2
desperately trying to pass codeclimate #9004
noi5e Jan 19, 2021
ca93f14
desperately trying to pass codeclimate #9004
noi5e Jan 19, 2021
f44a6bb
desperately trying to pass codeclimate #9004
noi5e Jan 19, 2021
2f04589
oops, switched ternary operator #9004
noi5e Jan 19, 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
8 changes: 3 additions & 5 deletions app/assets/javascripts/dragdrop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

aka ES6 destructuring assignment

Copy link
Member

Choose a reason for hiding this comment

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

Oohhhh!!! 🤗

e.preventDefault();
if (params.hasOwnProperty('dSelected')) {
$D.selected = params['dSelected'];
}
$E.initialize(params);
if (dSelected) { $D.selected = dSelected; }
$E.setState(textArea, preview);
});

$(this).fileupload({
Expand Down
50 changes: 27 additions & 23 deletions app/assets/javascripts/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,22 @@ $(function() {
// 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
// however, the editor is also used on pages with JUST ONE form, and no other comments, eg. /wiki/new & /wiki/edit, so this code needs to be reusable for that context
$('.rich-text-button').on('click', function(e) {
const params = getEditorParams(e.target); // defined in editorHelper.js
const { textArea, preview, dSelected } = getEditorParams(e.target); // defined in editorHelper.js
// assign dSelected
if (params.hasOwnProperty('dSelected')) {
$D.selected = params['dSelected'];
}
$E.initialize(params);
if (dSelected) { $D.selected = dSelected; }
$E.setState(textArea, preview);
const action = e.currentTarget.dataset.action // 'bold', 'italic', etc.
$E[action](); // call the appropriate editor function
});
});

$E = {
initialize: function(args) {
args = args || {}
// title
$E.title = $('#title')
// textarea
args['textarea'] = args['textarea'] || 'text-input'
$E.textarea = $('#'+args['textarea'])
$E.textarea.bind('input propertychange',$E.save)
// preview
args['preview'] = args['preview'] || 'preview-main'
$E.preview = $('#'+args['preview'])
initialize: function() {
// call setState with no parameters, aka. default parameters.
// default parameters point toward either:
// 1. the comment form at the bottom of multi-comment wikis/questions/research notes
// 2. the only editor form on /wiki/new and /wiki/edit
$E.setState();

marked.setOptions({
gfm: true,
Expand All @@ -44,6 +37,12 @@ $E = {
}
});
},
setState: function(textarea = 'text-input', preview = 'preview-main', title = 'title') {
$E.title = $('#' + title + 'title'); // not sure why this exists? seems like $E.title is always #title
$E.textarea = $('#' + textarea);
$E.textarea.bind('input propertychange', $E.save);
$E.preview = $('#' + preview);
},
is_editing: function() {
return ($E.textarea[0].selectionStart == 0 && $E.textarea[0].selectionEnd == 0)
},
Expand All @@ -54,14 +53,19 @@ $E = {
// preview
$E.preview = ($D.selected).find('.comment-preview').eq(0);
},
isRichTextEditor: function(url) {
// this RegEx matches three different cases where the legacy editor is still used:
// 1. /wiki/new
// 2. /wiki/{wiki name}/edit
// 3. /features/new
const legacyEditorPath = RegExp(/\/(wiki|features)(\/[^\/]+\/edit|\/new)/);
return !legacyEditorPath.test(url); // if we're not on one of these pages, we are using the rich-text editor.
},
// wraps currently selected text in textarea with strings a and b
wrap: function(a,b,args) {
// this RegEx is: /wiki/ + any char not "/" + /edit
const isWikiCommentPage = (/\/wiki\/[^\/]+\/edit/).test(window.location.pathname);
// we don't need to refresh $E's values if we're on a page with a single comment form, ie. /wiki/new or /wiki/edit
if (window.location.pathname !== "/wiki/new" && !isWikiCommentPage && $D.selected) {
this.refresh();
}
wrap: function(a, b, args) {
noi5e marked this conversation as resolved.
Show resolved Hide resolved
// 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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Ready to merge!

Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/editorHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ const getEditorParams = (targetDiv) => {
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'] = 'preview-main';
}
return params;
};
10 changes: 4 additions & 6 deletions app/views/comments/_edit.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@
</div>
<script type="application/javascript">
function setInit(id) {
var args = {
'textarea': 'c'+id+'text',
'preview': 'c'+id+'preview'
}
$E.initialize(args);
const textArea = 'c'+id+'text';
const preview = 'c'+id+'preview';
$E.setState(textArea, preview);
}

$('#c<%= comment.id%>div').on('dragover',function(e) {
Expand Down Expand Up @@ -115,7 +113,7 @@
});
</script>

<div class="well col-md-11" id="c<%= comment.id %>preview" style="background:white;display: none">
<div class="comment-preview well col-md-11" id="c<%= comment.id %>preview" style="background:white;display: none">
</div>

<div class="control-group">
Expand Down
2 changes: 1 addition & 1 deletion app/views/editor/_editor.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@

<script>
jQuery(document).ready(function() {
$E.initialize({})
$E.initialize();
})
</script>
2 changes: 1 addition & 1 deletion test/system/comment_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ def get_path(page_type, path)
page.find('#c' + edit_id_num + 'edit a', text: 'Preview').click
page.first('a', text: 'Preview').click
assert_selector('#c' + edit_id_num + 'preview img', count: 1)
assert_selector('.comment-preview img', count: 1)
assert_selector('#preview-reply-' + reply_id_num, count: 1)
end

test "#{page_type_string}: ctrl/cmd + enter comment publishing keyboard shortcut" do
Expand Down