-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Ask for a reason when reporting assistants #825
Conversation
@@ -46,7 +46,7 @@ | |||
role="presentation" | |||
tabindex="-1" | |||
bind:this={backdropEl} | |||
on:click={handleBackdropClick} | |||
on:click|stopPropagation={handleBackdropClick} |
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.
just to be sure, this doesn't affect other use cases of Modal
. Right ?
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.
I tested our other modals and it doesn't, but I think we needed this line anyway: otherwise when you have two modals, clicking the backdrop would close all of them instead of the topmost one
src/routes/settings/assistants/[assistantId]/ReportModal.svelte
Outdated
Show resolved
Hide resolved
src/routes/settings/assistants/[assistantId]/ReportModal.svelte
Outdated
Show resolved
Hide resolved
Co-authored-by: Victor Muštar <victor.mustar@gmail.com>
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.
lgtm !
Alright, going to check the slack integration in prod then! |
* Show modal asking for assistant report reason * fix disabled button styling * parse with zod * Update src/routes/settings/assistants/[assistantId]/ReportModal.svelte Co-authored-by: Victor Muštar <victor.mustar@gmail.com> * use a text area with max-w --------- Co-authored-by: Victor Muštar <victor.mustar@gmail.com>
Maxed reason to 128 chars, should work with the slack webhook integration.