-
Notifications
You must be signed in to change notification settings - Fork 331
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
apply first submission modal changes #584
Changes from 2 commits
71513d3
8431ad4
817451a
afb94dc
c8162b3
4cb0914
8a98ab4
7220cfc
aa41578
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,69 @@ | ||||||||||||||||||||||||
<template> | ||||||||||||||||||||||||
<div | ||||||||||||||||||||||||
class="border border-nt-blue-light bg-blue-50 dark:bg-notion-dark-light rounded-md p-2 overflow-hidden" | ||||||||||||||||||||||||
> | ||||||||||||||||||||||||
<div class="flex items-center w-full gap-2"> | ||||||||||||||||||||||||
<p class="select-all text-nt-blue flex-grow truncate overflow-hidden"> | ||||||||||||||||||||||||
<a | ||||||||||||||||||||||||
v-if="link" | ||||||||||||||||||||||||
:href="share_url" | ||||||||||||||||||||||||
target="_blank" | ||||||||||||||||||||||||
> | ||||||||||||||||||||||||
{{ share_url }} | ||||||||||||||||||||||||
</a> | ||||||||||||||||||||||||
<span v-else> | ||||||||||||||||||||||||
{{ share_url }} | ||||||||||||||||||||||||
</span> | ||||||||||||||||||||||||
</p> | ||||||||||||||||||||||||
<UButton | ||||||||||||||||||||||||
class="shrink-0" | ||||||||||||||||||||||||
size="sm" | ||||||||||||||||||||||||
icon="i-heroicons-clipboard-document" | ||||||||||||||||||||||||
label="Copy" | ||||||||||||||||||||||||
@click="copyToClipboard" | ||||||||||||||||||||||||
/> | ||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||
</template> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
<script setup> | ||||||||||||||||||||||||
import { computed, defineProps } from 'vue' | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const props = defineProps({ | ||||||||||||||||||||||||
form: { | ||||||||||||||||||||||||
type: Object, | ||||||||||||||||||||||||
required: true, | ||||||||||||||||||||||||
}, | ||||||||||||||||||||||||
link: { | ||||||||||||||||||||||||
type: Boolean, | ||||||||||||||||||||||||
default: false, | ||||||||||||||||||||||||
}, | ||||||||||||||||||||||||
extraQueryParam: { | ||||||||||||||||||||||||
type: String, | ||||||||||||||||||||||||
default: '', | ||||||||||||||||||||||||
}, | ||||||||||||||||||||||||
}) | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const { copy } = useClipboard() | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const share_url = computed(() => { | ||||||||||||||||||||||||
return props.extraQueryParam | ||||||||||||||||||||||||
? `${props.form.share_url}?${props.extraQueryParam}` | ||||||||||||||||||||||||
: props.form.share_url + props.extraQueryParam | ||||||||||||||||||||||||
}) | ||||||||||||||||||||||||
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. Fix the logic in the share_url computed property. The current implementation of the Please update the computed property as follows: const share_url = computed(() => {
- return props.extraQueryParam
- ? `${props.form.share_url}?${props.extraQueryParam}`
- : props.form.share_url + props.extraQueryParam
+ if (props.extraQueryParam) {
+ return `${props.form.share_url}?${props.extraQueryParam}`
+ }
+ return props.form.share_url
}) This change ensures that the 📝 Committable suggestion
Suggested change
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. This is a great suggestion 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. @JhumanJ, glad you found the suggestion helpful! (_/) |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
function copyToClipboard() { | ||||||||||||||||||||||||
if (import.meta.server) | ||||||||||||||||||||||||
return | ||||||||||||||||||||||||
copy(share_url.value) | ||||||||||||||||||||||||
if (props.form.visibility == 'draft') { | ||||||||||||||||||||||||
useAlert().warning( | ||||||||||||||||||||||||
'Copied! But other people won\'t be able to see the form since it\'s currently in draft mode', | ||||||||||||||||||||||||
) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
else { | ||||||||||||||||||||||||
useAlert().success('Copied!') | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+56
to
+68
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. 🛠️ Refactor suggestion Consider implementing async/await and error handling in copyToClipboard. While the function logic is correct, it could be improved by implementing async/await and adding error handling as suggested in the past review comments. Consider updating the function as follows: async function copyToClipboard() {
if (import.meta.server)
return
try {
await copy(share_url.value)
if (props.form.visibility === 'draft') {
useAlert().warning(
'Copied! But other people won\'t be able to see the form since it\'s currently in draft mode',
)
} else {
useAlert().success('Copied!')
}
} catch (error) {
useAlert().error('Failed to copy URL')
}
} This change adds error handling and ensures the alert is shown only after the copy operation is complete. |
||||||||||||||||||||||||
</script> | ||||||||||||||||||||||||
|
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.
Have you tested this...? Not working not the right URL.
To fix:
OpnForm/api/app/Models/Forms/Form.php
Lines 147 to 154 in 8431ad4
OpnForm/api/app/Http/Resources/FormResource.php
Lines 28 to 29 in 8431ad4