- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 135
fix cancel on edit #2592
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
base: master
Are you sure you want to change the base?
fix cancel on edit #2592
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import Button from 'react-bootstrap/Button' | ||
|  | ||
| export default function ConfirmModal ({ | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose this can be used directly instead of also creating  Maybe this can be called generically  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 @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. | ||
| onConfirm, | ||
| message = 'Are you sure?', | ||
| confirmText = 'yep', | ||
| confirmVariant = 'info', | ||
| onClose | ||
| }) { | ||
| return ( | ||
| <> | ||
| <p className='fw-bolder'>{message}</p> | ||
| <div className='d-flex justify-content-end'> | ||
| <Button | ||
| variant={confirmVariant} | ||
| onClick={() => { | ||
| onConfirm() | ||
| onClose() | ||
| }} | ||
| > | ||
| {confirmText} | ||
| </Button> | ||
| </div> | ||
| </> | ||
| ) | ||
| } | ||
|  | ||
| export function CancelWorkConfirm ({ onConfirm, onClose }) { | ||
| return ( | ||
| <ConfirmModal | ||
| message='Are you sure? You will lose your work' | ||
| confirmText='yes' | ||
| confirmVariant='danger' | ||
| onConfirm={onConfirm} | ||
| onClose={onClose} | ||
| /> | ||
| ) | ||
| } | ||
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.
is this somehow related to #2568?
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 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.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.
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
Uh oh!
There was an error while loading. Please reload this page.
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.
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)