-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Discussion] Add Unique HTML IDs to Comment Forms to Fix Image Upload Cross-Wiring Issues #8926
Closed
2 tasks done
Milestone
Comments
This was referenced Jan 3, 2021
noi5e
added a commit
to noi5e/plots2
that referenced
this issue
Jan 5, 2021
jywarren
pushed a commit
that referenced
this issue
Jan 6, 2021
* generate unique HTML IDs for image upload .dropzones & inputs * update system tests referring to #fileinput * move get_dropzone_id logic to application_helper.rb #8926 * change comments to HTML comments * fix indents to pass linter
noi5e
added a commit
to noi5e/plots2
that referenced
this issue
Jan 11, 2021
noi5e
added a commit
to noi5e/plots2
that referenced
this issue
Jan 11, 2021
jywarren
pushed a commit
that referenced
this issue
Jan 11, 2021
manchere
pushed a commit
to manchere/plots2
that referenced
this issue
Feb 13, 2021
* generate unique HTML IDs for image upload .dropzones & inputs * update system tests referring to #fileinput * move get_dropzone_id logic to application_helper.rb publiclab#8926 * change comments to HTML comments * fix indents to pass linter
manchere
pushed a commit
to manchere/plots2
that referenced
this issue
Feb 13, 2021
* add new image upload system tests * rename comment fixtures * assign D.selected in start; rewrite drop assignment publiclab#8926 * specify E.refresh conditional call in E.wrap publiclab#8926
lagunasmel
pushed a commit
to lagunasmel/plots2
that referenced
this issue
Mar 2, 2021
* generate unique HTML IDs for image upload .dropzones & inputs * update system tests referring to #fileinput * move get_dropzone_id logic to application_helper.rb publiclab#8926 * change comments to HTML comments * fix indents to pass linter
lagunasmel
pushed a commit
to lagunasmel/plots2
that referenced
this issue
Mar 2, 2021
* add new image upload system tests * rename comment fixtures * assign D.selected in start; rewrite drop assignment publiclab#8926 * specify E.refresh conditional call in E.wrap publiclab#8926
reginaalyssa
pushed a commit
to reginaalyssa/plots2
that referenced
this issue
Oct 16, 2021
* generate unique HTML IDs for image upload .dropzones & inputs * update system tests referring to #fileinput * move get_dropzone_id logic to application_helper.rb publiclab#8926 * change comments to HTML comments * fix indents to pass linter
reginaalyssa
pushed a commit
to reginaalyssa/plots2
that referenced
this issue
Oct 16, 2021
* add new image upload system tests * rename comment fixtures * assign D.selected in start; rewrite drop assignment publiclab#8926 * specify E.refresh conditional call in E.wrap publiclab#8926
billymoroney1
pushed a commit
to billymoroney1/plots2
that referenced
this issue
Dec 28, 2021
* generate unique HTML IDs for image upload .dropzones & inputs * update system tests referring to #fileinput * move get_dropzone_id logic to application_helper.rb publiclab#8926 * change comments to HTML comments * fix indents to pass linter
billymoroney1
pushed a commit
to billymoroney1/plots2
that referenced
this issue
Dec 28, 2021
* add new image upload system tests * rename comment fixtures * assign D.selected in start; rewrite drop assignment publiclab#8926 * specify E.refresh conditional call in E.wrap publiclab#8926
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Creating a fresh issue page for this complex bug that appears in many forms. See #8670 and #8478 for two examples of this bug in action. Also see current PR #8897 (awaiting review) that fixes a small piece of this bug.
Comment editor functions are cross-wired:
Why This Happens
The JS scripts in
/views/comments/_edit.html.erb
and/app/assets/dragdrop.js
have variables like$D.selected
and$E.textarea
that point toward the comment form that the user last interacted with. However, these are often pointing to the wrong form. They need to be updated every time the user takes particular actions: (toggling a form open, entering text, uploading an image).Comment Forms: Uploading Images
div.dropzone
wraps aninput#fileinput
.div#c1234div
.dragdrop.js
mentioned above.input#fileinput
here, similar to the one above but distinct.Why This Matters
Ideally, we could write code that sets
$D.selected
to the appropriatetextarea
whenever the user takes one of the actions above. To reiterate,$D.selected
is a kind of place marker for the editor to know where to put the user's changes (an image link, bold text, etc.)In practice,
$D.selected
would rely one.target
from a click or drop event handler. However, logginge.target
shows that it's often inaccurate. In the example below, I upload an image in comment 22, ande.target
(and the image upload) points toward comment 7!Potential causes for this:
input
s anddiv class='dropzone'
s with the same IDs and classes running around.Next Steps
I committed some changes in Add Unique HTML IDs to .dropzones, #fileinputs #8927 setting unique IDs on all of the elements diagrammed above. I have a feeling that this will solve the problem ofe.target
being inaccurate. Another fix could potentially be rewriting the event handlers so that they're only searching for click or drop events within certain parts of the DOM tree.$D.selected
whenever a user clicks to upload images. [PR open: Fix CLICK-to-Upload Image Cross-Wiring Issues #8987](This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)
The text was updated successfully, but these errors were encountered: