fix cancel on edit #2592

pory-gone · 2025-10-01T16:52:32Z

Description

Implemented confirmation prompt when canceling comment edit.

Created reusable ConfirmModal component with presets for different confirmation types.
Added confirmation modal logic to comment.js eoggleEdit function that checks for unsaved changes in localStorage.
Updated both comment.js and reply.js to use the new reusable component.
Added storageKeyPrefix to comment-edit.js to properly track form changes

Screenshots

2025-10-01-18-08-22.mp4

Additional Context

The implementation follows the exact same pattern used in reply.js to maintain consistency.
the confirmation logic is handled at the parent component level comment.js
This implementation DOES NOT fix the multi-tab problem highlighted by @adlai, a broader refactor of the editing system would be needed, which goes beyond the scope of this issue.

Checklist

Are your changes backward compatible? Please answer below:
Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
7/10

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
NaN

Did you introduce any new environment variables? If so, call them out explicitly here:
No

Did you use AI for this? If so, how much did it assist you?
I used AI to analyze the existing codebase and find interested files

Note

^{Cursor Bugbot is generating a summary for commit c4b2bd6. Configure here.}

adlai · 2025-10-02T16:54:50Z

Thank you for the shout-out; and was my own limited human bafflement by the nightmare of modern client-side HTML5 codebases helpful in any way, given that you also had an AI tool review the entire codebase?

Soxasora

Pretty cool! Left some nitpicks, but other than that it works correctly.

Also thank you for providing a centralized modal for confirmations

Soxasora · 2025-10-12T14:42:43Z

components/confirm-modal.js

@@ -0,0 +1,50 @@
+import Button from 'react-bootstrap/Button'
+
+export default function ConfirmModal ({


I suppose this can be used directly instead of also creating CancelWorkConfirm and DeleteConfirm (which you didn't use but I get why you created it)

Maybe this can be called generically ObstacleModal but I'm just thinking out loud, there's no need to do this.

which you didn't use

@pory-gone Please don't add code that is dead on arrival

You're more than welcome to refactor something, but please not while trying to fix something and not in a way that it's half-done.

Soxasora · 2025-10-12T14:52:50Z

components/comment.js

+                  toggleEdit={e => {
+                    if (edit) {
+                      const editText = window.localStorage.getItem(`comment-edit-${item.id}-text`)
+                      if (editText?.trim() && editText.trim() !== item.text.trim()) {


The first trim (editText?.trim()) can be avoided, so that any change can count in the check.

Mhh, I agree with @Soxasora

I don't think any user will care about losing an edit full of whitespace, but I think the inconsistency when the prompt shows up might confuse users. It might not be clear that it additionally checks if there's only whitespace.

ekzyis · 2025-10-19T22:16:25Z

components/comment-edit.js

          }}
          schema={commentSchema}
          onSubmit={onSubmit}
+          storageKeyPrefix={`comment-edit-${comment.id}`}


is this somehow related to #2568?

This use local storage to save a draft of the user's edit, the draft is used to verify that there is actually text to be modified as reply.js. If you prefer not to use local storage we could open a modal when you want to close the edit.

Not really needed and kind of out of the scope of the PR but I appreciate the fact that it makes the draft experience consistent in all typing modes, and it's also the way textboxes are handled in other places.

So that's why I didn't comment on it. If useless, it can just be removed and the obstacle modal would be there only to guard the state setting

the draft is used to verify that there is actually text to be modified as reply.js

Ahh ok, that makes sense!

Sorry, I could have figured this out if I just looked at the other code. I tend to now just immediately ask if something is really related if it's not immediately clear since I wasted too much time in my life trying to figure out how something in someone else's code is related to a ticket or bug, just for it to end up not being related at all (and maybe even have a bug)

ekzyis

Relevant changes look good, but I did not test them yet.

Please remove the dead code, then I will take another look, thanks

ekzyis · 2025-10-20T16:58:34Z

components/confirm-modal.js

@@ -0,0 +1,50 @@
+import Button from 'react-bootstrap/Button'
+
+export default function ConfirmModal ({


which you didn't use

@pory-gone Please don't add code that is dead on arrival

You're more than welcome to refactor something, but please not while trying to fix something and not in a way that it's half-done.

ekzyis · 2025-10-20T17:06:25Z

components/comment.js

+                  toggleEdit={e => {
+                    if (edit) {
+                      const editText = window.localStorage.getItem(`comment-edit-${item.id}-text`)
+                      if (editText?.trim() && editText.trim() !== item.text.trim()) {


Mhh, I agree with @Soxasora

I don't think any user will care about losing an edit full of whitespace, but I think the inconsistency when the prompt shows up might confuse users. It might not be clear that it additionally checks if there's only whitespace.

ekzyis · 2025-10-20T17:10:05Z

components/comment-edit.js

          }}
          schema={commentSchema}
          onSubmit={onSubmit}
+          storageKeyPrefix={`comment-edit-${comment.id}`}


the draft is used to verify that there is actually text to be modified as reply.js

Ahh ok, that makes sense!

Sorry, I could have figured this out if I just looked at the other code. I tend to now just immediately ask if something is really related if it's not immediately clear since I wasted too much time in my life trying to figure out how something in someone else's code is related to a ticket or bug, just for it to end up not being related at all (and maybe even have a bug)

pory-gone · 2025-10-23T17:20:44Z

Thanks for your feedback! I removed the DeleteConfirm component from confirm-modal.js as it was unused and the first .trim() check in comment.js as suggested by @Soxasora - now any change (including whitespace) will trigger the confirmation

fix cancel on edit

c4b2bd6

pory-gone marked this pull request as ready for review October 1, 2025 16:55

Soxasora approved these changes Oct 12, 2025

View reviewed changes

ekzyis added the bug label Oct 19, 2025

ekzyis reviewed Oct 19, 2025

View reviewed changes

ekzyis requested changes Oct 20, 2025

View reviewed changes

removing dead code and improved correct one

2af9e45

pory-gone requested a review from ekzyis October 23, 2025 19:02

		@@ -0,0 +1,50 @@
		import Button from 'react-bootstrap/Button'

		export default function ConfirmModal ({

Uh oh!

Uh oh!

fix cancel on edit #2592

Are you sure you want to change the base?

fix cancel on edit #2592

Conversation

pory-gone commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots

Additional Context

Checklist

Uh oh!

adlai commented Oct 2, 2025

Uh oh!

Soxasora left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekzyis Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekzyis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekzyis Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pory-gone commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pory-gone commented Oct 1, 2025 •

edited

Loading

ekzyis Oct 20, 2025 •

edited

Loading

ekzyis Oct 20, 2025 •

edited

Loading