-
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
Add Unique Preview Button IDs #9012
Add Unique Preview Button IDs #9012
Conversation
@@ -123,25 +123,17 @@ $E = { | |||
generate_preview: function(id,text) { | |||
$('#'+id)[0].innerHTML = marked(text) | |||
}, | |||
toggle_preview: function (comment_id = null) { | |||
toggle_preview: function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I searched through plots2
: it seems as if toggle_preview
is never called with a comment_id
parameter. I could be wrong about that(?) For now, I am deleting this unused code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, and comment previews are toggled with a different method? Ok!
@@ -208,7 +208,7 @@ en: | |||
preview_topics: "This preview is not yet published and is not part of any topics yet." | |||
_comments: | |||
comments: "Comments" | |||
post_comment: "Post comment" | |||
post_comment: "Post Comment" | |||
post_placeholder: "Help the author refine their post, or point them at other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a small thing, but I noticed this when writing system tests.
Codecov Report
@@ Coverage Diff @@
## main #9012 +/- ##
=======================================
Coverage ? 82.03%
=======================================
Files ? 100
Lines ? 5933
Branches ? 0
=======================================
Hits ? 4867
Misses ? 1066
Partials ? 0 |
Code Climate has analyzed commit 373f349 and detected 0 issues on this pull request. View more on Code Climate. |
Tests are passing now (they were referring to the old IDs and classes). Ready to merge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Merging now...thanks evreyone 🎉 |
* rename method for making comment element IDs * add unique IDs + new classes for preview buttons * capitalize comment in wiki Post comment form * change references to new preview button classes & IDs * change preview ID to match comment editor's * change CSS for new comment preview IDs * change preview button CSS selector
* rename method for making comment element IDs * add unique IDs + new classes for preview buttons * capitalize comment in wiki Post comment form * change references to new preview button classes & IDs * change preview ID to match comment editor's * change CSS for new comment preview IDs * change preview button CSS selector
* rename method for making comment element IDs * add unique IDs + new classes for preview buttons * capitalize comment in wiki Post comment form * change references to new preview button classes & IDs * change preview ID to match comment editor's * change CSS for new comment preview IDs * change preview button CSS selector
* rename method for making comment element IDs * add unique IDs + new classes for preview buttons * capitalize comment in wiki Post comment form * change references to new preview button classes & IDs * change preview ID to match comment editor's * change CSS for new comment preview IDs * change preview button CSS selector
Will leave more detailed inline comments in the reviews for each file.
(This issue is part of the larger Comment Editor Overhaul Project with Outreachy. Refer to Planning Issue #9069 for more context)