Fix Image Drag & Drop Cross-Wiring in Edit Comment Form #8897

noi5e · 2020-12-27T06:09:46Z

This is a PR in progress! Don't merge yet. Eventually it will fix #8670. Right now, just looking for feedback from @jywarren.

Essentially this will fix a bug that occurs when uploading images to the 'Edit Comment' form. Sometimes the image will end up in the main comment form, not the edit comment form. (Read the issue comments for more details)

So far, I've written a system test that consistently reproduces this behavior (see Files Changed).

I'm writing up a comment right now summarizing my research, why I think this bug happens, and questions about where to go from here.

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

gitpod-io · 2020-12-27T06:09:50Z

codecov · 2020-12-27T06:15:12Z

Codecov Report

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

@@           Coverage Diff           @@
##             main    #8897   +/-   ##
=======================================
  Coverage        ?   81.94%           
=======================================
  Files           ?      100           
  Lines           ?     5955           
  Branches        ?        0           
=======================================
  Hits            ?     4880           
  Misses          ?     1075           
  Partials        ?        0

noi5e · 2020-12-28T09:13:31Z

Did some more research, I've refined my understanding of what happens here. The questions above still apply so please check them out!

What Scripts Run Where

I was mistaken before about what runs where. Looking at views I believe that certain JS scripts won't run on certain comment forms. dragdrop.js is required for the main comment form and the reply form. However the edit comment form has its own separate image upload script which starts here in /app/views/_edit.html.erb:

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

Line 72 in cd59bfc

$('#c<%= comment.id%>div').fileupload({

So I think my assumption before that dragdrop.js would run on an edit comment form is wrong I believe.

Scenario: Bug

If I sit down and break down the case where the bug occurs, I think it goes like this:

drop an image into the main comment form.

dragdrop.js's drop eventListener sets $D.selected to the main comment form's div.comment-form-wrapper.
dragdrop.js's callback for successful uploads sets $E.textarea to main comment form's textarea

open an edit comment form for c22 and drop an image there.

#c22div's drop eventListener gets called from within the comment edit form partial's embedded JS (/app/views/_edit.html.erb)
this calls setInit, which sets $E.textarea to #c22text textarea.

_edit.html.erb has a done callback for fileupload:

it calls $E.wrap which is supposed to wrap the image URL and put it in to the $E.textarea.
$E.wrap checks to see if we're currently on a wiki page or not.
If it's not a wiki, then it calls $E.refresh.
$E.refresh checks to see if $D.selected exists or not.
- $D.selected does exist! It's been set to the main comment form (see step 1).
- $E.textarea is set to $D.selected's textarea, and that's where the image URL string is placed, not the edit comment form like we want.

Those are my working assumptions so far. I think it explains why the error doesn't happen on wikis. Going to map out the success case (where the URL goes into the edit comment form like expected), and then from there think about how to fix this one.

noi5e · 2020-12-28T09:39:01Z

Yeah, I think my scenario for the bug is accurate. Here's the scenario for the success case:

Scenario: Success

open an edit comment form for c22 and drop an image there.

turns out that _edit.html.erb's setInit function gets called from /app/views/notes/_comment.html.erb:

plots2/app/views/notes/_comment.html.erb

Line 56 in cd59bfc

    
           <a aria-label="Edit comment" class="btn btn-outline-secondary btn-sm" id="edit-comment-btn" href="javascript:void(0)" onClick="$('#c<%= comment.cid %>edit').toggle();$('#c<%= comment.cid %>show').toggle();setInit(<%= comment.cid %>);">

this means that $E.textarea becomes #c22div textarea after opening the edit form.
#c22div's drop eventListener gets called from within the comment edit form partial's embedded JS (/app/views/_edit.html.erb)
this calls setInit AGAIN, which sets $E.textarea to #c22text textarea again for good measure.

_edit.html.erb has a done callback for fileupload:

it calls $E.wrap which is supposed to wrap the image URL and put it in to the $E.textarea.
$E.wrap checks to see if we're currently on a wiki page or not.
If it's not a wiki, then it calls $E.refresh.
$E.refresh checks to see if $D.selected exists or not.
- $D.selected does NOT exist!
- $E.textarea remains as #c22text, and the image URL string goes into the right comment box. Hooray! 🙌

…tener (publiclab#8670)

noi5e · 2020-12-29T08:13:43Z

app/views/comments/_edit.html.erb

+
+        $('#c<%= comment.id%>div').bind('drop',function(e) {
+          e.preventDefault();
+          $D.selected = $(e.target).closest('div.comment-form-wrapper').eq(0);


Notice this additional line!

I wrapped everything in _edit.html.erb in a div.comment-form-wrapper.

The other major change is that now $D.selected is assigned in this eventListener bound to #c1div.

A major part of the drag-and-drop crosswiring is fixed. My tests (including the new one I wrote for this issue) are passing. I had to rewrite some of the system tests in this most recent push: 'edit comment' and 'delete comment' rely on .comment-form-wrapper for some reason.

Oops, the tests aren't passing. I'll take a closer look tomorrow.

this is awesome

app/views/comments/_form.html.erb

jywarren · 2020-12-29T21:10:14Z

This is looking awesome, i am thinking that some of the keys to bear in mind may be:

let's try to pragmatically solve the bug you so excellently identified with the test, first, then move on to a more structural change. this lets us trust the test more for showing us that the solution is valid
for the later, more structural solution, we might prefer a solid unique ID instead of relying on .closest('div.comment-form-wrapper') to get us the correct textarea. Are there cases where we aren't getting, or using, a truly unique ID? For example, is there any re-use of ids, or anywhere we are not able to assign a unique ID for some reason?
the MANY ways of choosing an image is in part because people have hugely varying preferences and expectations for image upload. There's actually another -- some people try to copy-paste images 😅 which somehow actually works... sometimes!?? I think that later, once we resolve this particular bug and are more confident in our tests protecting these various scenarios, we might try to consolidate the handling of all of these as much as possible through the same code pathways.
Our tests should try to show the 'ideal' way to write code in this area, as well as walk people through all the variations we need to support. You can imagine how the many /types/ of comments, replies, and places to post them, plus AJAX vs. non, plus click to select vs. drag/drop plus clipboard, all contribute to situations where coders not DEEPLY familiar may fix a small issue but inadvertently break other variants of this interaction. Our tests should not only prevent this by testing all the relevant variants, but should model "good usage" such as a) assigning everything a unique ID, b) consistently using that unique ID to address different forms. Hopefully this will make our code more maintainable and legible to folks going forward!
It's such a pain to hunt through all the places where code lives here. I think choosing a good pattern and sticking to it could help - like, some minimal code to establish the state (unique ID, is it a wiki, etc) and then passing it into a standard comment form construction function may be a good approach. but you'll know best as you continue... this has simply gotten pretty overgrown over the years!

I hope some of this helps, i know you are very likely already thinking about a lot of this. You're doing an excellent job and unraveling some really subtle and messy threads... and reducing technical debt! No need to tackle every optimization or ideal code structure at once, but i feel that making smaller pragmatic changes, adding tests to document our steps, and then slowly and iteratively working towards a better and more legible structure is probably the best way to go. Good work unraveling this without getting overwhelmed by the scope!!!

jywarren · 2020-12-29T21:26:27Z

Do image drag-and-drop upload and image select-from-file upload both run through the same code above? I think they do, but am not sure. Using either one will reproduce the same bug.

OK, you found the duplicated code already but I think it could be good to look at the blame view of

plots2/app/assets/javascripts/dragdrop.js

Line 44 in cd59bfc

$(this).fileupload({

vs.

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

Line 72 in cd59bfc

$('#c<%= comment.id%>div').fileupload({

and see if it was apparent to the authors that this code existed in 2 places. I tend to think they look similar enough that we should consolidate it in dragdrop.js -- but not in this PR, let's think about that as a separate task!

Are there any other parts in the code that would be good for me to research?

I guess just that given that we use JS to build forms once new comments are added via AJAX, we might start to consider if we want that to be the only way we create comment forms -- that is, consolidate with a buildCommentForm(options) method so we can standardize that and test it as well -- just like we did with addComment() before!

Any strategy suggestions for bug-fixing given all the above?

Definitely do things one at a time in separate PRs. And, once you've really grasped what is /supposed/ to happen, write a descriptively-named test for it -- even if it's a pretty small thing! At least add a comment to the test to explain the step -- a good example might be // $D.selected is used to represent _____ within the tests. That way the test suite both establishes a "known area" you can rely upon, while also generating documentation -- for yourself as much as for any future coder!

I always feel that it's hard to keep a complex system in my head, so i try not to. If it takes a lot of time to recover a mental state -- get oriented in the code -- that's a sign of code which should be broken down into simpler more descriptive parts, using function names and tests to map things out and become more legible.

The wrap function I mentioned at the end of my last comment is interesting. Basically this is the function that's called in order to wrap the image link text. For some reason, it checks to see if it's on a wiki page, and it doesn't refresh if so. This could be why the test passes on wiki pages, but neither on questions nor on notes.

Here, I'd look at the blame view for that wiki switching code and try to trace back who added it and why... i know wiki comments were added much later than others, but i don't totally understand why they'd behave differently in this scenario!

OK, hope this helps!!!! 👍 again, great work!!!

…osswiring' into bug/edit-comment-select-image-crosswiring

app/assets/javascripts/dragdrop.js

codeclimate · 2021-01-08T01:03:04Z

Code Climate has analyzed commit 42f2e05 and detected 0 issues on this pull request.

View more on Code Climate.

noi5e · 2021-01-08T01:27:24Z

😌 Phew. I think this is it. Leaving my last few notes.

I gave unique IDs to comment textareas. Now there are only two elements with the ID #text-input:

main comment form (at bottom of page) on pages with multiple comments
textarea on pages to create new note, question, and wiki (because we reuse the same form for comments and node creation)

unique textarea IDs unfortunately revealed more bugs. i'm going to break out these things into new issues because this PR is gigantic already:

Bug 1: I saw for the first time that reply comment forms are cross-wired with each other, in production.
1. Go to a page with multiple comments (this is in production), and open the 2nd or 3rd comment (not the 1st one!)
2. Upload an image by clicking on the dropzone button in the toolbar.
3. The image will very often end up in the 1st comment reply form (open it by clicking on Reply to this comment...)
4. Our system tests weren't catching this bug, because they only test for the 1st comment on a page.
Bug 2: A bug with the test, image drag & drop into reply form on wiki comments was failing.
1. This used to succeed for similar reasons as bug 1. The test used to pass because $E was initialized to the first reply comment form on the page. With unique IDs, it has to resolve to the main form so the assertion for image in reply form fails after I added unique textarea IDs.
Bug 3: When I tried to fix bug 2, a new bug manifested. This test started to fail.
1. This was failing with my bugfix for bug 2 because the comment form partial is reused for creating new wikis.
2. If $E is initialized with parameters, it has to account for both scenarios: 1) a page with multiple comments on it 2) a create new wiki page.

I fixed bugs 2 and 3 in dragdrop.js's drop listener by adding a conditional that checks for both scenarios. It is kind of hacky, and I still want to eventually rewrite this so that we don't have to initialize $E here, but for now I think it works really well!

noi5e · 2021-01-08T01:45:32Z

@jywarren This one's ready to merge! It passes all the tests, and I've checked it out locally as well by playing with image upload on comments and /wiki/new. I feel very confident it's not creating any new bugs.

Sorry this is such a big PR with so many long notes. I probably could've broken out the new textarea IDs piece into another PR, but I really didn't think that was going to bring up more complications. (My bugfixes would break a test, which I would then fix, which would break more tests, etc.) I will make smaller PRs from hereon out.

Just to summarize, here is what this PR is doing:

assign unique IDs to comment textareas
rewrite drop listener in dragdrop.js so that $E can be re-initialized for two different scenarios:
- the appropriate comment form on a multi-comment page
- #text-input on a /wiki/new page
/comments/_edit.html.erb, edit comment form partial:
- wrap the entire edit comment form in a div.comment-form-wrapper
- assign $D.selected to the div.comment-form-wrapper when something is dropped onto an edit form's dropzone
comment system tests:
- rewrite drop_in_dropzone helper so images can be dragged & dropped on multiple .dropzones
- renamed some system tests for clarity
- rewrote old 'comment image upload' test to focus on the main form, not reply form
  - i will add the reply form test back in my next PR to fix the bug i mentioned above (Bug 1)
- add new system test: image DRAG & DROP into EDIT form isn't cross-wired with MAIN form
- rewrote tests that selected #text-input to instead select the appropriate .text-input

noi5e · 2021-01-08T01:58:14Z

app/assets/javascripts/dragdrop.js

+  $('.dropzone').on('drop',function(e) {
+    $D.selected = $(e.target).closest('div.comment-form-wrapper').eq(0);
+    e.preventDefault();
+    let params = {};


This part is new, and significant. Rather than just comment out $E, I rewrote this initialization call to account for two different scenarios:

pages where there's only one comment form (ie. /wiki/new). we want this to be initialized to #text-input.

pages with multiple comments-- here we want $E.textarea to be the closest textarea to the drop event.

Like I've said, I do want to refactor this method and others so that we don't have to $E.initialize, but I feel like this is a very good compromise for now.

jywarren · 2021-01-08T22:20:38Z

OK, i'm going through this comment by comment, so taking notes in my comment as i go... reading through #8897 (comment), I wanted to note that some of the confusion from $E.initialize() is really because a) it's very very old code, remade many times, and b) it doesn't explain itself well -- what are we initializing? Why is it that we can do it without any explicit parameters? Shouldn't we assign its output to a variable whose state is permanent, like var replyCommentEditor2 = new CommentEditor({...}); instead of just firing off generic non-specific initialize calls left and right? 😵

I will also admit that maybe my brain was fatigued from thinking through all these things in pretty intense detail.

This is definitely a sign that we should try to compartmentalize the logic more. Nobody should be juggling all these things in their head at once -- it's a sign of bad design! 😄

The big catch is that click-to-upload in reply comment forms breaks

Did you find this via a test failure? If not, perhaps we should ensure it's covered, because our longer-term desire to redesign the initialization system may mean we're gonna start tearing things out to rebuild... and we'll want good test coverage to do that with confidence. But no worries, we won't do this in this PR most likely!

Basically after this point, you did great... your guess at breaking a test, your docs re: 3 different bugs... all good.

Our system tests weren't catching this bug, because they only test for the 1st comment on a page.

great catch! This can be saved for later, let's just be organized about remembering things to circle back to.

Re: hacky fix, that's 👍 totally fine as you point out we'll come back here to fix more soon... just leave a note to self!

In general, to avoid such large PRs, what if we have the principle that we split out any bugs we find that aren't related to the core function of this PR, into their own issues (or even just to checkboxes on the planning issue for now).

not creating any new bugs

exactly!

OK, and finished reading your comments, and all the rest sound super. I really appreciate your including your reasonings here, as I think it'll provide helpful context if we need to circle back at some point.

And in general, +1000000 rewriting $E.initialize({}); to be more legible and to manage state better. In general, rather than an ambiguous initialize function, we should have a constructor function which sets things up, creates a persistent state, and returns an instance of a self-managing class that has uniqueness and retains the relationships between different HTML components with high specificity.

Merging this now, MANY thanks for such careful work, and congrats! This is also bringing into much sharper focus the architectural refinements we hope for down the road, their relationship to existing code, tests, and legibility!

🎉 🎉 🎉 🎉 🎉

jywarren · 2021-01-08T22:23:03Z

app/assets/javascripts/dragdrop.js

+    $D.selected = $(e.target).closest('div.comment-form-wrapper').eq(0);
+    e.preventDefault();
+    let params = {};
+    if ($D.selected.hasOwnProperty(0)) {


This logic is a bit obscure, hasOwnProperty(0) but might be improved with an explanatory comment. I think it also shows us that $D may also need refactoring along similar lines to $E.

And, apologies as I believe I was originally responsible for both $D and $E - and wow, i've grown so much as a coder in the ~10 years or so since they were written!!! And, to be frank, so has JavaScript!

Noted, and agreed, will write some comments here to explain!

I did notice you wrote a lot of $D and $E, no worries! 😄 I'm seeing what goes into growing and maintaining a 10+ year codebase, which is such a valuable learning experience. It's a real achievement that this repo has been growing for so long!

It's kind of neat how much of this work is historical. Like it was interesting to realize that the editor code was written for creating new posts first, then repurposed for comments. I understood the code a lot more then.

jywarren · 2021-01-08T22:24:11Z

test/system/comment_test.rb

@@ -186,15 +186,70 @@ def get_path(page_type, path)
      page.assert_selector('#preview img', count: 1)
    end

+    # sometimes if edit and reply/main comment forms are open, 


Nice comments here! Thanks!

jywarren

Awesome. Last review all good.

closest()

The extra state-management and specificity of a future rewrite of $E should make using this kind of DOM searching unnecessary, too!

OK, merging now!!

noi5e · 2021-01-09T18:53:50Z

Thanks @jywarren again for sifting through this very large PR! It was very satisfying to git merge upstream/main today.

In general, to avoid such large PRs, what if we have the principle that we split out any bugs we find that aren't related to the core function of this PR, into their own issues (or even just to checkboxes on the planning issue for now).

Definitely, yeah, will do this in the future! I'm starting to understand how to break up really complex issues into smaller more manageable PRs. I think before I was a little intimidated at the thought of managing merge conflicts, but now I see that's way more desirable than the alternative.

Did you find this via a test failure (click-to-upload in reply comment forms breaks)? If not, perhaps we should ensure it's covered, because our longer-term desire to redesign the initialization system may mean we're gonna start tearing things out to rebuild... and we'll want good test coverage to do that with confidence. But no worries, we won't do this in this PR most likely!

Hmm, I don't think so. All the system tests were passing at that point, because we don't have that kind of test coverage.

Definitely noted! This will be covered in the next few PRs, which are focused on click-to-upload issues.

In general, rather than an ambiguous initialize function, we should have a constructor function which sets things up, creates a persistent state, and returns an instance of a self-managing class that has uniqueness and retains the relationships between different HTML components with high specificity.

Noted!

Will make a small follow-up PR to leave comments, and change the bind to on that Sagarpreet noticed.

…dlers to .on (#8982) * change .bind eventHandlers to on #8897 * write explanatory comments #8897

…ab#8897) * add test for comment image crosswiring * wrap everything in comment-form-wrapper, assign .selected in drop listener (publiclab#8670) * delete defunct function, fix indentation & spacing publiclab#8670 * rewrite tests that rely on comment-form-wrapper * remove deprecated code, comments; fix indent * initialize on pageload (publiclab#8670) * rewrite for multiple file inputs * change test to focus on image drag & drop publiclab#8670 * restore E.initialize call in drop publiclab#8670 * undo E.initialize call publiclab#8670 * assign unique IDs to comment form textareas publiclab#8670 * change ID selectors to class ones to match new IDs publiclab#8670 * fix indentation publiclab#8670 * change ID reference to class reference publiclab#8670 * initialize E with parameters in drop listener publiclab#8670 * update tests for new text-input selectors publiclab#8670 * change ID text-input reference to class ref publiclab#8670 * change text-input ID to class publiclab#8670 * drop: initialize E with params publiclab#8670 * remove semicolon

…dlers to .on (publiclab#8982) * change .bind eventHandlers to on publiclab#8897 * write explanatory comments publiclab#8897

…ab#8897) * add test for comment image crosswiring * wrap everything in comment-form-wrapper, assign .selected in drop listener (publiclab#8670) * delete defunct function, fix indentation & spacing publiclab#8670 * rewrite tests that rely on comment-form-wrapper * remove deprecated code, comments; fix indent * initialize on pageload (publiclab#8670) * rewrite for multiple file inputs * change test to focus on image drag & drop publiclab#8670 * restore E.initialize call in drop publiclab#8670 * undo E.initialize call publiclab#8670 * assign unique IDs to comment form textareas publiclab#8670 * change ID selectors to class ones to match new IDs publiclab#8670 * fix indentation publiclab#8670 * change ID reference to class reference publiclab#8670 * initialize E with parameters in drop listener publiclab#8670 * update tests for new text-input selectors publiclab#8670 * change ID text-input reference to class ref publiclab#8670 * change text-input ID to class publiclab#8670 * drop: initialize E with params publiclab#8670 * remove semicolon

…dlers to .on (publiclab#8982) * change .bind eventHandlers to on publiclab#8897 * write explanatory comments publiclab#8897

…ab#8897) * add test for comment image crosswiring * wrap everything in comment-form-wrapper, assign .selected in drop listener (publiclab#8670) * delete defunct function, fix indentation & spacing publiclab#8670 * rewrite tests that rely on comment-form-wrapper * remove deprecated code, comments; fix indent * initialize on pageload (publiclab#8670) * rewrite for multiple file inputs * change test to focus on image drag & drop publiclab#8670 * restore E.initialize call in drop publiclab#8670 * undo E.initialize call publiclab#8670 * assign unique IDs to comment form textareas publiclab#8670 * change ID selectors to class ones to match new IDs publiclab#8670 * fix indentation publiclab#8670 * change ID reference to class reference publiclab#8670 * initialize E with parameters in drop listener publiclab#8670 * update tests for new text-input selectors publiclab#8670 * change ID text-input reference to class ref publiclab#8670 * change text-input ID to class publiclab#8670 * drop: initialize E with params publiclab#8670 * remove semicolon

…dlers to .on (publiclab#8982) * change .bind eventHandlers to on publiclab#8897 * write explanatory comments publiclab#8897

…ab#8897) * add test for comment image crosswiring * wrap everything in comment-form-wrapper, assign .selected in drop listener (publiclab#8670) * delete defunct function, fix indentation & spacing publiclab#8670 * rewrite tests that rely on comment-form-wrapper * remove deprecated code, comments; fix indent * initialize on pageload (publiclab#8670) * rewrite for multiple file inputs * change test to focus on image drag & drop publiclab#8670 * restore E.initialize call in drop publiclab#8670 * undo E.initialize call publiclab#8670 * assign unique IDs to comment form textareas publiclab#8670 * change ID selectors to class ones to match new IDs publiclab#8670 * fix indentation publiclab#8670 * change ID reference to class reference publiclab#8670 * initialize E with parameters in drop listener publiclab#8670 * update tests for new text-input selectors publiclab#8670 * change ID text-input reference to class ref publiclab#8670 * change text-input ID to class publiclab#8670 * drop: initialize E with params publiclab#8670 * remove semicolon

…dlers to .on (publiclab#8982) * change .bind eventHandlers to on publiclab#8897 * write explanatory comments publiclab#8897

add test for comment image crosswiring

63d200c

noi5e added testing issues are usually for adding unit tests, integration tests or any other tests for a feature outreachy labels Dec 27, 2020

noi5e added this to the [Outreachy] Comment Editor Winter 2020 milestone Dec 27, 2020

noi5e requested review from jywarren, sagarpreet-chadha and cesswairimu December 27, 2020 06:09

This comment has been minimized.

Sign in to view

noi5e mentioned this pull request Dec 27, 2020

OLD CLOSED Comment Editor Overhaul Issue #8775

Closed

noi5e changed the title ~~Fix Image Upload Cross-Wiring in Edit Comment Form~~ Bug: Fix Image Upload Cross-Wiring in Edit Comment Form Dec 27, 2020

This comment has been minimized.

Sign in to view

noi5e added 3 commits December 29, 2020 00:08

wrap everything in comment-form-wrapper, assign .selected in drop lis…

f34bc6b

…tener (publiclab#8670)

delete defunct function, fix indentation & spacing publiclab#8670

68f820c

rewrite tests that rely on comment-form-wrapper

9814c32

noi5e requested a review from a team as a code owner December 29, 2020 08:10

noi5e commented Dec 29, 2020

View reviewed changes

app/views/comments/_form.html.erb Show resolved Hide resolved

This comment has been minimized.

Sign in to view

noi5e added 4 commits December 30, 2020 02:06

remove deprecated code, comments; fix indent

2892710

initialize on pageload (publiclab#8670)

24589fa

rewrite for multiple file inputs

fc8b950

change test to focus on image drag & drop publiclab#8670

5a6a114

noi5e changed the title ~~Bug: Fix Image Upload Cross-Wiring in Edit Comment Form~~ Bug: Fix Image Drag & Drop Cross-Wiring in Edit Comment Form Dec 30, 2020

Merge remote-tracking branch 'origin/bug/edit-comment-select-image-cr…

8e67269

…osswiring' into bug/edit-comment-select-image-crosswiring

codeclimate bot reviewed Jan 8, 2021

View reviewed changes

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

remove semicolon

42f2e05

noi5e requested review from jywarren and sagarpreet-chadha January 8, 2021 01:47

noi5e commented Jan 8, 2021

View reviewed changes

jywarren reviewed Jan 8, 2021

View reviewed changes

jywarren approved these changes Jan 8, 2021

View reviewed changes

jywarren merged commit d38da1c into publiclab:main Jan 8, 2021

noi5e deleted the bug/edit-comment-select-image-crosswiring branch January 9, 2021 18:25

noi5e added a commit to noi5e/plots2 that referenced this pull request Jan 9, 2021

change .bind eventHandlers to on publiclab#8897

fd606ba

noi5e added a commit to noi5e/plots2 that referenced this pull request Jan 9, 2021

write explanatory comments publiclab#8897

fe2c155

noi5e mentioned this pull request Jan 9, 2021

Add $D.selected Comment Documentation, Change .bind eventHandlers to .on #8982

Merged

jywarren pushed a commit that referenced this pull request Jan 10, 2021

Feature: Add $D.selected Comment Documentation, Change .bind eventHan…

626c26c

…dlers to .on (#8982) * change .bind eventHandlers to on #8897 * write explanatory comments #8897

noi5e mentioned this pull request Jan 23, 2021

Comment Editor Overhaul Project [Outreachy] #9069

Open

noi5e changed the title ~~Bug: Fix Image Drag & Drop Cross-Wiring in Edit Comment Form~~ Fix Image Drag & Drop Cross-Wiring in Edit Comment Form Jan 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Image Drag & Drop Cross-Wiring in Edit Comment Form #8897

Fix Image Drag & Drop Cross-Wiring in Edit Comment Form #8897

noi5e commented Dec 27, 2020 •

edited

Loading

gitpod-io bot commented Dec 27, 2020 •

edited

Loading

codecov bot commented Dec 27, 2020 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

noi5e commented Dec 28, 2020 •

edited

Loading

noi5e commented Dec 28, 2020

This comment has been minimized.

noi5e Dec 29, 2020

noi5e Dec 29, 2020

noi5e Dec 29, 2020

jywarren Dec 29, 2020

This comment has been minimized.

jywarren commented Dec 29, 2020

jywarren commented Dec 29, 2020

codeclimate bot commented Jan 8, 2021

noi5e commented Jan 8, 2021 •

edited

Loading

noi5e commented Jan 8, 2021

noi5e Jan 8, 2021

jywarren commented Jan 8, 2021

jywarren Jan 8, 2021

noi5e Jan 9, 2021

jywarren Jan 8, 2021

jywarren left a comment

noi5e commented Jan 9, 2021

Fix Image Drag & Drop Cross-Wiring in Edit Comment Form #8897

Fix Image Drag & Drop Cross-Wiring in Edit Comment Form #8897

Conversation

noi5e commented Dec 27, 2020 • edited Loading

gitpod-io bot commented Dec 27, 2020 • edited Loading

codecov bot commented Dec 27, 2020 • edited Loading

Codecov Report

This comment has been minimized.

This comment has been minimized.

noi5e commented Dec 28, 2020 • edited Loading

What Scripts Run Where

Scenario: Bug

noi5e commented Dec 28, 2020

Scenario: Success

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

jywarren commented Dec 29, 2020

jywarren commented Dec 29, 2020

codeclimate bot commented Jan 8, 2021

noi5e commented Jan 8, 2021 • edited Loading

noi5e commented Jan 8, 2021

Choose a reason for hiding this comment

jywarren commented Jan 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren left a comment

Choose a reason for hiding this comment

noi5e commented Jan 9, 2021

noi5e commented Dec 27, 2020 •

edited

Loading

gitpod-io bot commented Dec 27, 2020 •

edited

Loading

codecov bot commented Dec 27, 2020 •

edited

Loading

noi5e commented Dec 28, 2020 •

edited

Loading

noi5e commented Jan 8, 2021 •

edited

Loading