ReviewNB / support

Issues and feature requests for ReviewNB
https://reviewnb.com
59 stars 8 forks source link

Pending comments are not saved? #25

Closed gsganden closed 5 years ago

gsganden commented 5 years ago

I made some comments but did not finish my review, and the next day those comments seemed to be gone. Is it possible to support pending comments like on GitHub, so that comments are saved across sessions even if the review is unfinished? If not, then would it be possible to change the UI to clarify that comments in unfinished reviews will not be saved?

amit1rrr commented 5 years ago

Storing unfinished review means we'll have to persist those comments on ReviewNB side. We do not want to store user's private repository data (including comments).

If not, then would it be possible to change the UI to clarify that comments in unfinished reviews will not be saved?

This is a good idea. Would some sort of a pop up if you try to close the tab with unfinished review be helpful?

gsganden commented 5 years ago

Storing unfinished review means we'll have to persist those comments on ReviewNB side. We do not want to store user's private repository data (including comments).

That makes sense. Would it be possible to use GitHub's pending comments functionality to avoid this problem?

This is a good idea. Would some sort of a pop up if you try to close the tab with unfinished review be helpful?

Yes. Maybe also use different language than "Start Review" and "Finish Review," because to me that language calls to mind GitHub's ability to store pending comments while you work on a review.

amit1rrr commented 5 years ago

Would it be possible to use GitHub's pending comments functionality to avoid this problem?

It's possible but will be a bit murky since people tend to use both interfaces (ReviewNB & GitHub) to create reviews. If we keep ReviewNB draft review in GitHub pending review state, users won't be able to -

  1. create a new review draft in GitHub (since GitHub only allows 1 pending review)
  2. won't be able to continue/finish the review in GitHub interface (because we add some additional metadata to comment (cell_index etc.))
  3. won't be able to read the draft review correctly in GitHub (because cell level comments you create in ReviewNB are not tied to a specific line as the comments created in GitHub)

For all these reasons, I'm not very inclined to piggyback on GitHub pending review. Maybe we can change this in future when users are using ReviewNB commenting exclusively & won't care if there's a draft review in GitHub UI.

Yes. Maybe also use different language than "Start Review" and "Finish Review," because to me that language calls to mind GitHub's ability to store pending comments while you work on a review.

Cool. I will have some kind of pop up show up to let people know that they might loose comments.

About wording, the intention was to keep it same so people can relate easily and know that this review will be posted as-is on GitHub. You're right that the mental model breaks for draft review. I will make the pop up wording explicitly clear so that people know this distinction before they loose their carefully crafted review comments.

Also, I am sorry you lost those comments you wrote!

gsganden commented 5 years ago

Thanks! The product is very helpful.

amit1rrr commented 5 years ago

@gsganden App is now updated to show confirmation dialog when the tab is getting closed or navigated away from with an unfinished review in it.

Most browsers (including Chrome) shows their own default confirm message in the pop up instead of our tailored message. In any case, it serves our purpose of giving user another chance to save their work.

Thank you for reporting.

Screenshot 2019-05-07 at 5 42 53 PM

DavidBAEpstein commented 3 years ago

I wamted to delete only one of my comments on ReviewNB while review was "Pending" ReviewNB promptly removed ALL my comments. Now I have to do the work over again

amit1rrr commented 3 years ago

@DavidBAEpstein I'm really sorry that you lost your draft comments. This can be really annoying.

I'm trying to reproduce this issue on my end but I'm unable to reproduce the problem. I created draft review with multiple comments & when I delete a single comment it removes only that single comment. Leaving the rest of the review intact.

Can you please re-try the steps that lead you to loose an entire review? If you're able to reproduce the issue can you please share the steps for the same?

DavidBAEpstein commented 3 years ago

Dear Amit,

Apologies. My comment was an error. When I looked more closely, I saw that most of my comments were still intact, and the deletions were almost certainly the result of me clicking the wrong boxes while panicking.

Is it possible for you to delete my comment? I feel a bit ashamed about this.

Thanks David

On 23 Oct 2021, at 13:25, Amit Rathi @.**@.>> wrote:

@DavidBAEpsteinhttps://github.com/DavidBAEpstein I'm really sorry that you lost your draft comments. This can be really annoying.

I'm trying to reproduce this issue on my end but I'm unable to reproduce the problem. I created draft review with multiple comments & when I delete a single comment it removes only that single comment. Leaving the rest of the review intact.

Can you please re-try the steps that lead you to loose an entire review? If you're able to reproduce the issue can you please share the steps for the same?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ReviewNB/support/issues/25#issuecomment-950144937, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFIQKBBPFBMAQIDUKJDS2CDUIKSTVANCNFSM4HJ6Z7PA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

amit1rrr commented 3 years ago

@DavidBAEpstein No worries. If you ever face any problem with ReviewNB, feel free to open a new issue here.