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

Comment Editor Overhaul Project [Outreachy] #9069

Open
noi5e opened this issue Jan 23, 2021 · 2 comments
Open

Comment Editor Overhaul Project [Outreachy] #9069

noi5e opened this issue Jan 23, 2021 · 2 comments
Assignees
@noi5e
Labels
feature explains that the issue is to add a new feature JavaScript outreachy planning Planning issues!
Milestone
[Outreachy] Comme...

Comments

@noi5e
Copy link
Contributor

noi5e commented Jan 23, 2021 •
edited
Loading

(Last updated Monday April 18th)

This issue summarizes my internship with Outreachy and Public Lab!

Also see the following GitHub Milestone.

Overtime!

Synopsis

The internship is technically wrapped up! I am popping in whenever reasonable finish a React revision to the commenting system.

🥝 Open PRs (merge from top-to-bottom) 🦁

  • disable main comment form while guest browsing ❗ [open PR: #11072]

Rewrite Comment Editor with React

I wrote a basic React version of the comment system in #9175.

Currently working on improvements to the React system in issue #9365, including:

  • system tests to measure performance
  • emoji reactions
  • comment preview
  • spam moderation
  • rich-text formatting

Write Issues

  • FTO issue: rename comment.add_comment to comment_from_email

Stretch Goals

  • new toolbar features:
    • h2 rich-text button provides drop-down options for h1, h3, h4, etc.
    • markdown features: blockquote, table, code, etc.
  • submit edit comment form with AJAX (ie. don't refresh the page)
  • improve speed to post comment: OLD CLOSED Comment Editor Overhaul Issue #8775 (comment)
    • particularly, delivering emails asynchronously, in a different thread

Late-Stage Internship Tasks:

  • write up thoughts on spinning out library into another repo, or general state of Comment Editor.
    • make diagrams
      • editor dependencies, ie. views and scripts within editor.js's ecosystem
      • state management layout
  • clean up system tests & fixtures
    • use node.add_comment where necessary to reduce redundant fixtures
    • refactor drop_in_dropzone helper method for efficiency
    • ensure that tests targeting reply forms are not using the 1st reply form

What I've Done

Rewrite Comment Editor with React

  • install webpacker & React (merged in #9176)
  • setup alternate React commenting system when visiting a research note with parameter ?react=true (merged in #9176)
  • create CRUD functionality for comments in React (merged in #9176)
  • make separate controller routes for react_create, react_update, etc. (merged in #9176)
  • add react proptypes for array and object props (merged in #9751)
  • implement useReducer for comment state management (merged in #9801)
  • implement useReducer for commentFormsVisibility (merged in #9862)
  • implement useReducer for textAreaValues (merged in #9862)

Refactor Comment Editor with Object-Oriented Programming

  • write $E.setState() method using ES6 default parameters (merged in #9035)
    • set preview state inside of $E.setState() calls (merged in #9035)
  • refactor editor.js with object-oriented principles (merged in #9104)
  • rewrite $E.setState & $E.constructor for single ID parameter (merged in #9107)
    • completely remove DOM-searching in state-setting functions! (merged in #9107)
    • remove handleClick() function and call when opening reply forms. (merged in #9107)
  • prune editor state management (merged in #9108)
    • remove $D.selected! (merged in #9108)
    • remove $E.refresh! (merged in #9108)
    • add unique IDs to image upload progress bars (merged in #9108)
  • refactor editor constructor with new parameter; write editor getters (merged in #9110)
    • change instances of $E.method(); to this.method(); (merged in #9110)
    • $E.initialize({ default main editor form ID }) - allows for different default form IDs (merged in #9110)
    • write getter methods to retrieve $E.textarea, $E.textAreaValue, and $E.preview (merged in #9110)

Bugfixes for Improved User Experience

  • don't show emojis from banned users (merged in #8875)
  • fix cross-wiring for edit/main drag-and-drop image upload (merged in #8897)
    • rewrite drop listener in dragdrop.js so that $E can be re-initialized for single & multi-form pages (merged in #8897)
    • wrap comments/_edit.html.erb in div.comment-form-wrapper, reassign $D.selected (merged in #8897)
  • fix cross-wiring for click to image upload (merged in #8987)
    • rewrite click and drop event handlers to set $D.selected (merged in #8987)
  • fix cross-wiring for rich-text (bold, italic) buttons (merged in #9011)
    • assign $E.textarea as e.target when clicking rich-text buttons. (merged in #9011)
  • show progress bars when clicking-to-upload images in edit comment form (merged in #9019
  • remove functionality for DRAG onto edit form's .dropzone to upload image (merged in #9118)
  • fix edit toggle preview bug I created (merged in #9067)
  • refactor save & recover methods to handle multiple comment forms (merged in #9132)
    • $E.setState when clicking on save, recover, or a textarea (merged in #9132)
    • change icon for recover button (merged in #9132)
  • fix rich-text & image upload in freshly posted comments (merged in #9162
  • create image upload functionality for comments vs. comment responses in create.js.erb (merged in #9195)
  • hide <textarea> when toggling preview in legacy editor (merged in #9234)

Code Reorganization & Deletion for Maintainability

  • rewrite script-loading of comment.js and dragdrop.js so they're not loaded once per comment (merged in #9037)
  • consolidate comment toolbar JS scripts; rename dragdrop.js to editorToolbar.js (merged in #9044)
  • delete _edit.html.erb in-template JavaScript; integrate with editor.js (merged in #9067)
    • delete <script> in _edit.html.erb and point it toward editor.js instead (merged in #9067)
    • remove setInit call on #edit-btn click (merged in #9067)
    • move $E.initialize to higher-level views (merged in #9067)
  • toggle_preview on #comment-form-body instead of #dropzone-large (merged in #9123)
  • merge comments/_edit.html.erb and comments/_form.html.erb into one partial (merged in #9183)
  • remove templates functionality from toolbar (merged in #9183)

Standardized and Made Unique HTML Element IDs

  • assign unique IDs to comment textareas (merged in #8897)
  • add unique HTML IDs to image upload elements (merged in #8927)
  • assign unique HTML IDs to rich-text toolbar buttons (merged in #8995)
  • add unique IDs and common classes for comment preview buttons (merged in #9012)
  • assign unique IDs and common classes to progress bars (merged in #9019)
  • create unique IDs; standardize <form> and preview IDs in reply and main comment forms (merged in #9062)
  • <form> IDs are: #comment-form-main, #comment-form-reply-123, and #comment-form-edit-123 (merged in #9062)
  • #preview-main and #preview123 are now: #comment-preview-main and #comment-preview-reply-123 (merged in #9062)
  • change edit's textarea and preview IDs: #comment-preview-edit-123 and #text-input-edit-123 (merged in #9067)

Researched & Wrote Issues

  • researched & closed this issue (liking comments): #5113
  • display notification for comment emoji reactions: #9071
  • opened save & recover discussion issue: #9131
  • make JavaScript filenames camelCased (broken down into 11 FTO issues) #9140
  • responsive design: header rich-text button disappears entirely #9342
  • researched and helped contributor with hide spam/moderated comment replies #8854
  • display notification when user likes/dislikes research note: #9353
  • animate save icon when editor saves comment form text: #9362
  • delete defunct templating code: #9919
  • close reply comment form after submission: #10000

Style Fixes

  • style fixes for image upload progress bars, drag & drop hover, edit comment forms (merged in #9019)
  • remove padding on reactions; remove superfluous grey bar; both mentioned here (merged in #9079)
  • increase margin space between comments (merged in #9079)
  • add tooltips for edit comment & emoji reactions (merged in #9079)
  • make Reply to comment link un-selectable (merged in #9079)
  • change hover style on emoji buttons (merged in #9079)
  • hide emoji reactions bar when opening edit comment form (merged in #9079)
  • tweak 'react to comment' button; swap in heart icon for smiley (merged in #9079)
  • show tooltips for top-right corner buttons in freshly posted comments; adjust comment form padding (merged in #9198)
  • adjust color and cursor: pointer; for toolbar buttons (merged in #9195)

Wrote System Tests

  • note: comment with addComment(comment_text, URL), assert comment_text exists in DOM (merged in #8809)
  • question: comment with addComment(comment_text, URL), assert comment_text exists in DOM (merged in #8801)
  • note: respond to existing comment with addComment(comment_text, URL, parent_id) (merged in #8844)
  • question: respond to existing comment with addComment(comment_text, URL, parent_id) (merged in #8845)
  • note: comment, then respond to freshly posted comment (merged in: #8860)
  • question: comment, then respond to freshly posted comment (merged in #8851)
  • consolidate and organize tests so we can run each type of test on each variant of the Comment Editor. (merged in #8860)
  • edit freshly posted comment
    • (I decided not to write this test, deciding it would be redundant. There's an already existing test that does this.)
    • Follow my research into this topic here
  • user posts fresh page, then posts fresh comment (merged in #8879)
  • open a mixture of "Reply to comment" boxes, and post comments in each. (merged in #8881)
  • image DRAG & DROP into EDIT form isn't cross-wired with MAIN form (merged in #8897)
  • rewrite drop_in_dropzone test helper to be reused for multiple drops (merged in #8897)
  • IMMEDIATE image CLICK to upload in REPLY comment form works (merged in #8987)
  • IMMEDIATE image CLICK to upload in EDIT comment form works (merged in #8987)
  • CROSS-WIRING: DROP image onto MAIN, CLICK on EDIT form's .dropzone (merged in #8987)
  • CROSS-WIRING: DROP image onto MAIN, CLICK on REPLY form's .dropzone (merged in #8987)
  • CROSS-WIRING: open EDIT comment form, rich-text input into REPLY comment form (merged in #9011)
  • show progress bars for DRAG & DROP image upload in MAIN comment form (merged in #9019)
  • show progress bars for CLICK to upload in EDIT comment form (merged in #9019)
  • reduce test coverage to save runtime: run comment tests for just comments on research notes (not wikis and questions) (merged in #9045)
  • toggle preview works in edit, reply, and main (merged in #9123)
  • save & recover buttons work (merged in #9139)
  • rich-text & image upload work in freshly posted comments (merged in #9162)
  • toggle preview works in /wiki/new (merged in #9234)
  • react comments: posting comment, posting reply to comment (merged in #9665)
  • react comments: edit a comment (merged in #9713)
  • react comments: delete a comment (merged in #9715)
  • split comment system tests into their own github actions CI workflow (merged in #9752)

Project Description from Public Lab

We have a collection of improvements, bugfixes, and refinements to our aging Comment editor subcomponent on the PublicLab.org website. The comment editor is used in various “variants” with shared code, complicating debugging (i.e. fixing the bug in one place can introduce a bug in another). This is a great case for system tests, where we can ensure behaviors are preserved, and for writing these tests before making any major code changes - as well as for identifying bugs by writing tests to demonstrate them, followed by submitting a fix within the same pull request, which gets the test to pass -- Test Driven Development. We have prioritized a list of bugs and refinements we’d like to fix first, and have then outlined a set of changes that could make the code more compact, readable, and maintainable to reduce future workload.
@noi5e noi5e added the feature explains that the issue is to add a new feature label Jan 23, 2021
@noi5e noi5e self-assigned this Jan 23, 2021
@noi5e noi5e mentioned this issue Jan 23, 2021
Closed
@noi5e
Copy link
Contributor Author

noi5e commented Jan 23, 2021

I copied the contents of my old planning issue over here... I thought it would be nice for the late-stage of the internship to have things be more readable here.

If you need to look at the old issue for any reason, it's here: #8775.

This was referenced Jan 23, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Closed
Closed
Merged
Merged
Closed
Merged
Merged
This was referenced Feb 14, 2021
Merged
Merged
@noi5e noi5e mentioned this issue Feb 24, 2021
Merged
@noi5e noi5e mentioned this issue Mar 23, 2021
Open
16 tasks
@noi5e noi5e mentioned this issue May 24, 2021
Merged
This was referenced Jun 1, 2021
Merged
Merged
Merged
noi5e added a commit to noi5e/plots2 that referenced this issue Jun 7, 2021
@noi5e
prune comment system tests publiclab#9069
95c39ad
@noi5e noi5e mentioned this issue Jun 7, 2021
Merged
@noi5e noi5e mentioned this issue Jun 15, 2021
Merged
@noi5e noi5e mentioned this issue Jun 27, 2021
Merged
@noi5e noi5e mentioned this issue Jul 19, 2021
Closed
5 tasks
noi5e added a commit to noi5e/plots2 that referenced this issue Jul 26, 2021
@noi5e
prune comment system tests publiclab#9069
a0aabec
noi5e added a commit to noi5e/plots2 that referenced this issue Jul 26, 2021
@noi5e
break comment system tests out to separate workflow publiclab#9069
41cbf10
noi5e added a commit to noi5e/plots2 that referenced this issue Jul 27, 2021
@noi5e
increase after_n_builds by 1 publiclab#9069
90dd92f
noi5e added a commit to noi5e/plots2 that referenced this issue Jul 28, 2021
@noi5e
change rails test command publiclab#9069
9c01f15
jywarren pushed a commit to noi5e/plots2 that referenced this issue Jul 30, 2021
@noi5e @jywarren
prune comment system tests publiclab#9069
ebf66e4
jywarren pushed a commit to noi5e/plots2 that referenced this issue Jul 30, 2021
@noi5e @jywarren
break comment system tests out to separate workflow publiclab#9069
8bf8ff3
jywarren pushed a commit to noi5e/plots2 that referenced this issue Jul 30, 2021
@noi5e @jywarren
increase after_n_builds by 1 publiclab#9069
a43b32d
jywarren pushed a commit to noi5e/plots2 that referenced this issue Jul 30, 2021
@noi5e @jywarren
change rails test command publiclab#9069
48fe4af
jywarren pushed a commit to noi5e/plots2 that referenced this issue Aug 2, 2021
@noi5e @jywarren
prune comment system tests publiclab#9069
e7e5e7e
jywarren pushed a commit to noi5e/plots2 that referenced this issue Aug 2, 2021
@noi5e @jywarren
break comment system tests out to separate workflow publiclab#9069
a5a9d30
jywarren pushed a commit to noi5e/plots2 that referenced this issue Aug 2, 2021
@noi5e @jywarren
increase after_n_builds by 1 publiclab#9069
c7a7d9d
jywarren pushed a commit to noi5e/plots2 that referenced this issue Aug 2, 2021
@noi5e @jywarren
change rails test command publiclab#9069
c9d0f5d
jywarren added a commit that referenced this issue Aug 2, 2021
@noi5e @jywarren
Prune Comment System Tests for Faster CI (#9752)
f775696 
* prune comment system tests #9069

* break comment system tests out to separate workflow #9069

* increase after_n_builds by 1 #9069

* change rails test command #9069

* removed extra "end"

Co-authored-by: jywarren <[email protected]>
reginaalyssa pushed a commit to reginaalyssa/plots2 that referenced this issue Oct 16, 2021
@noi5e @jywarren @reginaalyssa
Prune Comment System Tests for Faster CI (publiclab#9752)
02dc6c9 
* prune comment system tests publiclab#9069

* break comment system tests out to separate workflow publiclab#9069

* increase after_n_builds by 1 publiclab#9069

* change rails test command publiclab#9069

* removed extra "end"

Co-authored-by: jywarren <[email protected]>
billymoroney1 pushed a commit to billymoroney1/plots2 that referenced this issue Dec 28, 2021
@noi5e @jywarren @billymoroney1
Prune Comment System Tests for Faster CI (publiclab#9752)
30be463 
* prune comment system tests publiclab#9069

* break comment system tests out to separate workflow publiclab#9069

* increase after_n_builds by 1 publiclab#9069

* change rails test command publiclab#9069

* removed extra "end"

Co-authored-by: jywarren <[email protected]>
@noi5e noi5e mentioned this issue Mar 30, 2022
Merged
@noi5e noi5e mentioned this issue Apr 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@noi5e noi5e

Labels
feature explains that the issue is to add a new feature JavaScript outreachy planning Planning issues!
Projects
None yet
Milestone
[Outreachy] Comment Editor Winter 2020
Development

No branches or pull requests
2 participants
@jywarren @noi5e