Skip to content
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

feat: show document title for delete dialog - [DOC-387] #772

Merged
merged 4 commits into from
Dec 22, 2023

Conversation

adithyaakrishna
Copy link
Member

Signed-off-by: Adithya Krishna <adi@documenso.com>
Copy link

vercel bot commented Dec 19, 2023

Someone is attempting to deploy a commit to the Documenso Team Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Dec 19, 2023

Important

Auto Review Skipped

Auto 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 .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

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 ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@adithyaakrishna adithyaakrishna self-assigned this Dec 19, 2023
@github-actions github-actions bot added the apps: web Issues related to the webapp label Dec 19, 2023
Signed-off-by: Adithya Krishna <adi@documenso.com>
Copy link

vercel bot commented Dec 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stg-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 0:28am

dguyen
dguyen previously approved these changes Dec 20, 2023
@Mythie
Copy link
Collaborator

Mythie commented Dec 20, 2023

We should also display the title within the dialog rather than just the resulting toast notification as well, otherwise you're going to learn that you deleted the wrong document after the fact.

@adithyaakrishna
Copy link
Member Author

@Mythie How abt if we put it in a code block for the file name? or should we put it in double quotes?

Signed-off-by: Adithya Krishna <adi@documenso.com>
@adithyaakrishna
Copy link
Member Author

Updated the modal and the toast notification to show the documentTitle :)

Confirmation Modal:

Screenshot 2023-12-20 at 10 35 15

Toast Notification:

Screenshot 2023-12-20 at 10 35 22

@Mythie
Copy link
Collaborator

Mythie commented Dec 20, 2023

I would use quotes and do without the extra colour since it makes it look like a link 😄

@@ -81,7 +92,7 @@ export const DeleteDocumentDialog = ({
</DialogHeader>

{status !== DocumentStatus.DRAFT && (
<div className="mt-8">
<div className="mt-4">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing various top margins, it seems that this works well without any margin.

What do you think?

CleanShot 2023-12-20 at 17 41 37

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also felt the same, one of the previous PRs introduced the mt-8 and I felt the margin was too huge as well

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be me, I like having the action buttons separated from any kind of input/form elements to assist with misclicking

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example above, it doesn't seem like it would be easy to misclick. Or am I missing something? 👀

@ElTimuro
Copy link
Member

Updated the modal and the toast notification to show the documentTitle :)

Confirmation Modal:

Screenshot 2023-12-20 at 10 35 15 **Toast Notification:** Screenshot 2023-12-20 at 10 35 22

@adithyaakrishna The phrasing is a bit bulky, I'd prefer sth. like "Do you want to delete XXXX" or "Do you want to delete the document XXXXX"

@Mythie Mythie merged commit e9312ad into documenso:main Dec 22, 2023
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apps: web Issues related to the webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants