Added animation to save icon when user enters a comment text #9384

waridrox · 2021-03-29T08:15:35Z

Fixes #9362

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

Final working version -

Final.mp4

I've removed the console.log statements 😄.

@noi5e kindly review this. Thanks :)

gitpod-io · 2021-03-29T08:15:38Z

codecov · 2021-03-29T08:22:50Z

Codecov Report

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

❗ Current head e58228f differs from pull request most recent head 320ff86. Consider uploading reports for the commit 320ff86 to get more accurate results

@@           Coverage Diff           @@
##             main    #9384   +/-   ##
=======================================
  Coverage        ?   81.35%           
=======================================
  Files           ?       98           
  Lines           ?     5928           
  Branches        ?        0           
=======================================
  Hits            ?     4823           
  Misses          ?     1105           
  Partials        ?        0

noi5e · 2021-03-30T18:11:25Z

@waridrox Thanks so much for this, will give a detailed review and feedback later today, but honestly it looks pretty good.

One thing to note is that the save & recover system test needs to be updated (that's what's failing right now).

waridrox · 2021-03-31T06:21:18Z

One thing to note is that the save & recover system test needs to be updated (that's what's failing right now).

Oh!😅 Will take a look at this soon...

noi5e · 2021-03-31T07:04:27Z

Oh!😅 Will take a look at this soon...

@waridrox Yeah, that's a good idea! Updating the test isn't necessarily your responsibility (I wrote it), but I still recommend taking a look.

I'm thinking the test probably needs to be updated to wait for the debouncing to occur before it refreshes the page. I think it will be a relatively easy fix.

waridrox · 2021-04-04T16:42:22Z

My apologies, got a little busy lately 😅, @noi5e can you point me to the right file to implement the tests for debounce function. Further researching upon, I came across the sleep method in ruby which waits the program for a certain amount of time. Can we use this in the tests...?

RuthNjeri · 2021-04-15T19:16:34Z

Hi @waridrox, from the system tests here the failing test is

comment_test.rb

waridrox · 2021-04-23T19:29:23Z

Just when I though about adding tests, I realised we missed an edge case wherein due to the similar functional elements like the toolbar and the image upload section, the current changes also reflect if a Reply to a Comment is open along with Post Comment window. Here's an example for that case and I'm working on first fixing that!

edge-case.mp4

codeclimate · 2021-04-26T00:26:50Z

app/assets/javascripts/editor.js

+        let comment_temp_id = (document.activeElement.parentElement.parentElement.id);
+        let imager_bar = (document.activeElement.nextElementSibling.className);
+
+        $('#'+comment_temp_id).find('.btn-toolbar').find(".save-button").find("i").removeClass("fa fa-save").addClass("fas fa-sync fa-spin");


Similar blocks of code found in 2 locations. Consider refactoring.

app/assets/javascripts/editor.js

codeclimate · 2021-04-26T00:26:50Z

app/assets/javascripts/editor.js

+
+       //adding delay and revering the styles
+        setTimeout(() => {
+          $("#comment-form-main .btn-toolbar #save-button-main").find("i").removeClass("fas fa-sync fa-spin").addClass("fa fa-save");;


Unnecessary semicolon.

waridrox · 2021-04-26T00:34:07Z

Here's the fix for the edge case: reason being that each comment has its own unique id associated so earlier code was targeting all ids instead of the specific one that the user is currently making changes on.

edge.case.fix.mp4

Now working on testing...

codeclimate · 2021-04-26T09:03:17Z

app/assets/javascripts/editorToolbar.js

-      $E.save($E);
-    })
+      //preventing multiple clicks on the button
+      if(!e.detail || e.detail == 1) {


Expected '===' and instead saw '=='.

Isn't === supposed to check type as well 🤔. I think here == should suffice. But e.detail returns a typeof = number so I think we can use === here.

waridrox · 2021-04-27T15:29:14Z

The tests for save and recover button have been updated to account for debounce function and they pass now :)

@noi5e @RuthNjeri Kindly review this. Thanks :)

RuthNjeri · 2021-05-02T14:07:47Z

Hi @waridrox, what do you think about the other comments made by CodeClimate? Do you think you can address them? I see you responded to one...would you like to address the rest? We usually don't implement everything suggested by CodeClimate depending on the trade offs.

waridrox · 2021-05-02T14:29:02Z

Hi @waridrox, what do you think about the other comments made by CodeClimate? Do you think you can address them? I see you responded to one...would you like to address the rest? We usually don't implement everything suggested by CodeClimate depending on the trade offs.

Yes, @RuthNjeri, I've looked at the comments by CodeClimate, most of them being a repetition of one another 😅. But I don't see any problem in the issues that it is suggesting to be fixed -

Function attachSaveListener has 32 lines of code (exceeds 25 allowed). - This function also houses comments for explanations, but CodeClimate is also counting those in the number of lines.
Similar blocks of code found in 2 locations(✕4). Consider refactoring. - The code blocks do start similarly but eventually do different things at their completion.

gitpod-io · 2021-05-02T23:32:33Z

waridrox · 2021-05-11T19:56:07Z

@RuthNjeri @noi5e @jywarren pls review the changes. Thanks :)

test/system/comment_test.rb

app/assets/javascripts/editorToolbar.js

app/assets/javascripts/editor.js

waridrox · 2021-05-16T22:02:16Z

Sorry the tests did not run and instead failed...

pydevsg · 2021-05-17T07:24:49Z

Refactor the code @waridrox.
This will pass the test 😅

jywarren · 2021-05-17T22:33:42Z

If you'd like we can bypass the code climate suggestions! They are only suggestions but if you'd like to address any of them we can also wait, no rush!

waridrox · 2021-05-19T10:44:58Z

Hi @jywarren @pydevsg, I was talking about all the tests instead of just the code climate ones😅I've tried to address them but all of them seem fine to me...

RuthNjeri

Thanks for working on this @waridrox, it looks good to me, I think one more reviewer should approve this(@jywarren or @cesswairimu)...then it will be okay to merge 🚀

waridrox · 2021-05-23T20:14:22Z

After syncing the branch with upstream, unit tests seem to be failing...🤔

jywarren · 2021-05-25T21:14:00Z

Aha - for some reason i also saw this on #9665 (comment) and suggested a fix there - adding a sort to the test or to the main codebase, as i believe it's an ordering issue that can happen sometimes if we don't explicitly send a sort to the database. If you're able to open a new PR then possibly this and #9665 (comment) can both be rebased over it once we resolve that. In the meantime, i'll restart the test to see if we get past it that way. Thanks!

jywarren · 2021-05-30T00:52:40Z

Ok sorry @waridrox but now the unit tests upstream are fixed and just one more rebase should work. Many thanks for your patience!!!

codeclimate · 2021-05-30T08:27:06Z

Code Climate has analyzed commit 320ff86 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Duplication	4

View more on Code Climate.

waridrox · 2021-06-02T08:21:14Z

@jywarren Done!

jywarren · 2021-06-11T14:41:10Z

OK, merging now, and many thanks!!!

Regarding the revert button, i wonder if we should have saving add the text to an array of recent texts that have been saved. Then revert could just cycle back through that list.

jywarren · 2021-06-11T14:43:04Z

@waridrox @noi5e we could break out that idea of the array of comments saves into a new issue too! I think we'd have to be careful to flush the list... maybe only save 5 or 10 states, and/or delete much older ones or something? For example, what if someone types in something private, it saves, and they change their mind before posting it, they log out, someone else logs in, and they are able to recover the text.

jywarren · 2021-06-11T14:43:23Z

i.e. we could make it auto-delete texts older than an hour to mitigate this risk maybe.

jywarren · 2021-06-11T14:44:06Z

I know @waridrox is now working on other projects, though! So perhaps someone else from the @publiclab/soc team would be interested in taking this idea of fixing the comment revert button up?

…ab#9384) * Added animation to save icon when user enters a comment text * Edge case handled * Updated tests for save and recover buttons * Update editorToolbar.js * Unnecessary semicolon * ESLint typeof operator * let instead of var * Matched padding of saving text with file upload text

willow19 mentioned this pull request Apr 22, 2021

Weed out issues in the Candidate List that have already been worked on. #9529

Open

7 tasks

codeclimate bot reviewed Apr 26, 2021

View reviewed changes

waridrox mentioned this pull request May 5, 2021

Removed same text on comment reply form #9509

Merged

5 tasks

RuthNjeri reviewed May 16, 2021

View reviewed changes

test/system/comment_test.rb Show resolved Hide resolved

RuthNjeri reviewed May 16, 2021

View reviewed changes

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

RuthNjeri reviewed May 16, 2021

View reviewed changes

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

RuthNjeri reviewed May 16, 2021

View reviewed changes

app/assets/javascripts/editor.js Show resolved Hide resolved

waridrox requested a review from RuthNjeri May 16, 2021 12:16

waridrox closed this May 16, 2021

waridrox reopened this May 16, 2021

RuthNjeri approved these changes May 23, 2021

View reviewed changes

waridrox requested review from a team as code owners May 23, 2021 12:15

waridrox force-pushed the patch-animateSaveIcon branch 3 times, most recently from f1b318a to 338859f Compare May 23, 2021 20:10

waridrox and others added 8 commits May 30, 2021 13:09

Added animation to save icon when user enters a comment text

b148bc9

Edge case handled

b0e9a72

Updated tests for save and recover buttons

1e142f0

Update editorToolbar.js

a3affea

Unnecessary semicolon

8e41fd3

ESLint typeof operator

d3a49ab

let instead of var

c9305a5

Matched padding of saving text with file upload text

320ff86

waridrox force-pushed the patch-animateSaveIcon branch from e58228f to 320ff86 Compare May 30, 2021 08:25

RuthNjeri requested a review from jywarren June 11, 2021 09:35

RuthNjeri added the readytomerge label Jun 11, 2021

jywarren merged commit c76e485 into publiclab:main Jun 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added animation to save icon when user enters a comment text #9384

Added animation to save icon when user enters a comment text #9384

waridrox commented Mar 29, 2021 •

edited

Loading

gitpod-io bot commented Mar 29, 2021 •

edited

Loading

codecov bot commented Mar 29, 2021 •

edited

Loading

noi5e commented Mar 30, 2021

waridrox commented Mar 31, 2021 •

edited

Loading

noi5e commented Mar 31, 2021

waridrox commented Apr 4, 2021 •

edited

Loading

RuthNjeri commented Apr 15, 2021

waridrox commented Apr 23, 2021 •

edited

Loading

codeclimate bot Apr 26, 2021

codeclimate bot Apr 26, 2021

waridrox commented Apr 26, 2021 •

edited

Loading

codeclimate bot Apr 26, 2021

waridrox Apr 27, 2021 •

edited

Loading

waridrox commented Apr 27, 2021 •

edited

Loading

RuthNjeri commented May 2, 2021

waridrox commented May 2, 2021 •

edited

Loading

gitpod-io bot commented May 2, 2021

waridrox commented May 11, 2021 •

edited

Loading

waridrox commented May 16, 2021

pydevsg commented May 17, 2021

jywarren commented May 17, 2021

waridrox commented May 19, 2021 •

edited

Loading

RuthNjeri left a comment

waridrox commented May 23, 2021 •

edited

Loading

jywarren commented May 25, 2021

jywarren commented May 30, 2021

codeclimate bot commented May 30, 2021

waridrox commented Jun 2, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

Added animation to save icon when user enters a comment text #9384

Added animation to save icon when user enters a comment text #9384

Conversation

waridrox commented Mar 29, 2021 • edited Loading

gitpod-io bot commented Mar 29, 2021 • edited Loading

codecov bot commented Mar 29, 2021 • edited Loading

Codecov Report

noi5e commented Mar 30, 2021

waridrox commented Mar 31, 2021 • edited Loading

noi5e commented Mar 31, 2021

waridrox commented Apr 4, 2021 • edited Loading

RuthNjeri commented Apr 15, 2021

waridrox commented Apr 23, 2021 • edited Loading

codeclimate bot Apr 26, 2021

Choose a reason for hiding this comment

codeclimate bot Apr 26, 2021

Choose a reason for hiding this comment

waridrox commented Apr 26, 2021 • edited Loading

codeclimate bot Apr 26, 2021

Choose a reason for hiding this comment

waridrox Apr 27, 2021 • edited Loading

Choose a reason for hiding this comment

waridrox commented Apr 27, 2021 • edited Loading

RuthNjeri commented May 2, 2021

waridrox commented May 2, 2021 • edited Loading

gitpod-io bot commented May 2, 2021

waridrox commented May 11, 2021 • edited Loading

waridrox commented May 16, 2021

pydevsg commented May 17, 2021

jywarren commented May 17, 2021

waridrox commented May 19, 2021 • edited Loading

RuthNjeri left a comment

Choose a reason for hiding this comment

waridrox commented May 23, 2021 • edited Loading

jywarren commented May 25, 2021

jywarren commented May 30, 2021

codeclimate bot commented May 30, 2021

waridrox commented Jun 2, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

jywarren commented Jun 11, 2021

waridrox commented Mar 29, 2021 •

edited

Loading

gitpod-io bot commented Mar 29, 2021 •

edited

Loading

codecov bot commented Mar 29, 2021 •

edited

Loading

waridrox commented Mar 31, 2021 •

edited

Loading

waridrox commented Apr 4, 2021 •

edited

Loading

waridrox commented Apr 23, 2021 •

edited

Loading

waridrox commented Apr 26, 2021 •

edited

Loading

waridrox Apr 27, 2021 •

edited

Loading

waridrox commented Apr 27, 2021 •

edited

Loading

waridrox commented May 2, 2021 •

edited

Loading

waridrox commented May 11, 2021 •

edited

Loading

waridrox commented May 19, 2021 •

edited

Loading

waridrox commented May 23, 2021 •

edited

Loading