-
Notifications
You must be signed in to change notification settings - Fork 816
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
fix(webapp): reset delete document dialog #762
Conversation
@ubinatus is attempting to deploy a commit to the Documenso Team Team on Vercel. A member of the Team first needs to authorize it. |
Important Auto Review SkippedAuto reviews are limited to the following labels: coderabbit. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
Hey there, it looks like you haven't accepted our contributor license agreement yet. In order for us to accept your pull request we ask that you please fill out the CLA: |
Hey @ubinatus, thanks for the PR! The demo link doesn't work. Could you update the link, please? |
Hi @catalinpit! I've updated the description with a working public loom url. Sorry about that, it was toggled as a private link. |
Hey, thank you! In the meanwhile, I tested the changes and I came across an issue. If you try to delete a draft document, the That's because in the |
apps/web/src/app/(dashboard)/documents/delete-document-dialog.tsx
Outdated
Show resolved
Hide resolved
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.
Looks good. Thanks for the work!
useEffect(() => { | ||
if (open) { | ||
setInputValue(''); | ||
setIsDeleteEnabled(status === DocumentStatus.DRAFT ? true : false); |
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.
Can just be:
setIsDeleteEnabled(status === DocumentStatus.DRAFT);
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.
Agree! No need for that ternary. Done!
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Looks good to me, thanks 😃
This PR makes a small but useful tweak to the
DeleteDocumentDialog
. Now, the input field gets cleared whenever the dialog is opened. Here’s what’s changed:Clear Field After Deleting: After you delete something and open the dialog again, it won’t show the old, deleted text anymore. It’s clean and ready for the next delete.
Type Again to Confirm: If you type something but close the dialog without deleting, you’ll have to type it again next time. This way, it makes sure the user really mean to delete something and didn't do it by mistake.
Demo Link:
See the old vs. new in action here: https://www.loom.com/share/80eca0d3b1994f7cbcab6f222db2dbfe?sid=ebc6135c-345e-4640-b395-daff190a96e7
It’s a small change, but it makes the delete process safer and smoother.