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.

Sign up for GitHub

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

Jump to bottom

Form Editor v2.5 #599

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Form Editor v2.5 #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c33a221
Form Editor v2.5
chiragchhatrala Oct 21, 2024
fa6e4c0
Remove log debug
JhumanJ Oct 23, 2024
9c5c773
Merge branch 'main' into 10ba6-form-editor-v2.5
JhumanJ Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion client/components/forms/FileInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
</div>
<div
v-else
:style="inputStyle"
class="flex flex-col w-full items-center justify-center transition-colors duration-40"
:class="[
{'!cursor-not-allowed':disabled, 'cursor-pointer':!disabled,
Expand All @@ -29,12 +30,18 @@
theme.fileInput.spacing.horizontal,
theme.fileInput.spacing.vertical,
theme.fileInput.fontSize,
theme.fileInput.minHeight
theme.fileInput.minHeight,
{'border-red-500 border-2':hasError},
'focus:outline-none focus:ring-2'
]"
tabindex="0"
role="button"
:aria-label="multiple ? 'Choose files or drag here' : 'Choose a file or drag here'"
@dragover.prevent="uploadDragoverEvent=true"
@dragleave.prevent="uploadDragoverEvent=false"
@drop.prevent="onUploadDropEvent"
@click="openFileUpload"
@keydown.enter.prevent="openFileUpload"
>
<div class="flex w-full items-center justify-center">
<div
Expand Down
3 changes: 1 addition & 2 deletions client/components/forms/TextAreaInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
'!cursor-not-allowed !bg-gray-200': disabled,
},
]"
class="resize-y"
class="resize-y block"
:name="name"
:style="inputStyle"
:placeholder="placeholder"
Expand Down Expand Up @@ -63,7 +63,6 @@ export default {
props: {
...inputProps,
maxCharLimit: {type: Number, required: false, default: null},
showCharLimit: {type: Boolean, required: false, default: false},
},

setup(props, context) {
Expand Down
3 changes: 2 additions & 1 deletion client/components/forms/TextInput.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
:maxlength="maxCharLimit"
@change="onChange"
@keydown.enter.prevent="onEnterPress"
@focus="onFocus"
@blur="onBlur"
>

<template
Expand Down Expand Up @@ -74,7 +76,6 @@ export default {
max: {type: Number, required: false, default: null},
autocomplete: {type: [Boolean, String, Object], default: null},
maxCharLimit: {type: Number, required: false, default: null},
showCharLimit: {type: Boolean, required: false, default: false},
pattern: {type: String, default: null},
},

Expand Down
2 changes: 1 addition & 1 deletion client/components/forms/components/InputLabel.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
class="input-label"
:class="[
theme.default.label,
{ 'uppercase text-xs': uppercaseLabels, 'text-sm': !uppercaseLabels },
{ 'uppercase text-xs': uppercaseLabels, 'text-sm/none': !uppercaseLabels },
]"
>
<slot>
Expand Down
3 changes: 2 additions & 1 deletion client/components/forms/components/InputWrapper.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
<has-error
v-if="hasValidation && form"
:form="form"
:field="name"
:field-id="name"
:field-name="label"
/>
</slot>
</div>
Expand Down
30 changes: 26 additions & 4 deletions client/components/forms/components/VSelect.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
:class="[
theme.SelectInput.input,
theme.SelectInput.borderRadius,
{ '!ring-red-500 !ring-2 !border-transparent': hasError, '!cursor-not-allowed dark:!bg-gray-600 !bg-gray-200': disabled },
{
'!ring-red-500 !ring-2 !border-transparent': hasError,
'!cursor-not-allowed dark:!bg-gray-600 !bg-gray-200': disabled,
'focus-within:ring-2 focus-within:ring-opacity-100 focus-within:border-transparent': !hasError
},
inputClass
]"
>
Expand All @@ -19,12 +23,14 @@
aria-haspopup="listbox"
aria-expanded="true"
aria-labelledby="listbox-label"
class="cursor-pointer w-full flex-grow relative"
class="cursor-pointer w-full flex-grow relative focus:outline-none"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Accessibility concern with focus outline removal.

While the new focus and blur event listeners improve focus state tracking, removing the default focus outline (focus:outline-none) can negatively impact accessibility for keyboard users.

Consider keeping the focus outline or replacing it with a custom focus indicator that meets accessibility standards. For example:

class="cursor-pointer w-full flex-grow relative focus:outline-none focus:ring-2 focus:ring-blue-500"

The addition of @focus and @blur event listeners is a good improvement for managing focus state.

Also applies to: 32-33

:class="[
theme.SelectInput.spacing.horizontal,
theme.SelectInput.spacing.vertical
]"
@click="toggleDropdown"
@focus="onFocus"
@blur="onBlur"
>
<div
class="flex items-center"
Expand Down Expand Up @@ -237,12 +243,13 @@ export default {
allowCreation: {type: Boolean, default: false},
disabled: {type: Boolean, default: false}
},
emits: ['update:modelValue', 'update-options'],
emits: ['update:modelValue', 'update-options', 'focus', 'blur'],
data() {
return {
isOpen: false,
searchTerm: '',
defaultValue: this.modelValue ?? null
defaultValue: this.modelValue ?? null,
isFocused: false
}
},
computed: {
Expand Down Expand Up @@ -311,11 +318,26 @@ export default {
}
return this.modelValue === value
},
onFocus(event) {
this.isFocused = true
this.$emit('focus', event)
},

onBlur(event) {
this.isFocused = false
this.$emit('blur', event)
},

toggleDropdown() {
if (this.disabled) {
this.isOpen = false
} else {
this.isOpen = !this.isOpen
if (this.isOpen) {
this.onFocus()
} else {
this.onBlur()
}
}
if (!this.isOpen) {
this.searchTerm = ''
Expand Down
10 changes: 10 additions & 0 deletions client/components/forms/useFormInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ export function useFormInput(props, context, options = {}) {
return wrapperProps
})

const onFocus = (event) => {
context.emit('focus', event)
}

const onBlur = (event) => {
context.emit('blur', event)
}

// Watch for changes in props.modelValue and update the local content
watch(
() => props.modelValue,
Expand All @@ -104,5 +112,7 @@ export function useFormInput(props, context, options = {}) {
hasValidation,
hasError,
inputWrapperProps,
onFocus,
onBlur,
}
}
45 changes: 30 additions & 15 deletions client/components/forms/validation/HasError.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,65 @@
<transition name="fade">
<div
v-if="errorMessage"
class="has-error text-sm text-red-500 -bottom-3"
class="has-error text-xs text-red-500 mt-1"
v-html="errorMessage"
/>
</transition>
</template>

<script>
export default {
name: "HasError",
name: 'HasError',
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Quote Usage Detected for Component Names

Multiple Vue component names use both single and double quotes. Please standardize quote usage across all component names to maintain consistency.

🔗 Analysis chain

LGTM: Consistent quote usage

The change from double quotes to single quotes for the component name is a good stylistic choice.

To ensure consistency across the project, you may want to run the following command:

This will help identify any inconsistencies in quote usage for component names across your Vue files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for inconsistent quote usage in Vue component names
rg -t vue "name: [\"'].*[\"']" --stats

Length of output: 207

Script:

#!/bin/bash
# Check for inconsistent quote usage in Vue component names by targeting .vue files
rg -g '*.vue' 'name: [\"'].*[\"']' --stats

Length of output: 130

Script:

#!/bin/bash
# Check for inconsistent quote usage in Vue component names by targeting .vue files
rg -g '*.vue' 'name: ["\'].*["\']' --stats

Length of output: 93

Script:

#!/bin/bash
# Check for inconsistent quote usage in Vue component names by targeting .vue files
rg -g '*.vue' "name: [\"'].*[\"']" --stats

Length of output: 24795

props: {
form: {
type: Object,
required: true,
},
field: {
fieldId: {
type: String,
required: true,
},
fieldName: {
type: String,
required: false,
},
},
computed: {
errorMessage() {
if (!this.form || !this.form.errors || !this.form.errors.any())
if (!this.form.errors || !this.form.errors.any())
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null check for form.errors

The current check could throw an error if form.errors is undefined. Consider adding a null check.

-if (!this.form.errors || !this.form.errors.any())
+if (!this.form?.errors?.any())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!this.form.errors || !this.form.errors.any())
if (!this.form?.errors?.any())

return null
const subErrorsKeys = Object.keys(this.form.errors.all()).filter(
(key) => {
return key.startsWith(this.field) && key !== this.field
return key.startsWith(this.fieldId) && key !== this.fieldId
},
)
const baseError =
this.form.errors.get(this.field) ??
(subErrorsKeys.length ? "This field has some errors:" : null)
let baseError
= this.form.errors.get(this.fieldId)
?? (subErrorsKeys.length ? 'This field has some errors:' : null)
// If no error and no sub errors, return
if (!baseError) return null
if (!baseError)
return null

return `<p class="text-red-500">${baseError}</p><ul class="list-disc list-inside">${subErrorsKeys.map(
(key) => {
return "<li>" + this.getSubError(key) + "</li>"
},
)}</ul>`
// Check if baseError starts with "The {field.name} field" and replace if necessary
if (baseError.startsWith(`The ${this.fieldName} field`)) {
baseError = baseError.replace(`The ${this.fieldName} field`, 'This field')
}

const coreError = `<p class='text-red-500'>${baseError}</p>`
if (subErrorsKeys.length) {
return coreError + `<ul class='list-disc list-inside'>${subErrorsKeys.map(
(key) => {
return `<li>${this.getSubError(key)}</li>`
},
)}</ul>`
}

return coreError
},
},
methods: {
getSubError(subErrorKey) {
return this.form.errors.get(subErrorKey).replace(subErrorKey, "item")
return this.form.errors.get(subErrorKey).replace(subErrorKey, 'item')
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider making error key replacement configurable

The hardcoded replacement of the key with 'item' might not be suitable for all contexts. Consider making this configurable through props.

+props: {
+  subErrorReplacement: {
+    type: String,
+    default: 'item'
+  }
+},
 methods: {
   getSubError(subErrorKey) {
-    return this.form.errors.get(subErrorKey).replace(subErrorKey, 'item')
+    return this.form.errors.get(subErrorKey).replace(subErrorKey, this.subErrorReplacement)
   }
 }

Committable suggestion was skipped due to low confidence.

},
},
}
Expand Down
10 changes: 6 additions & 4 deletions client/components/global/EditableTag.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
v-if="!editing"
:content="content"
>
<label class="cursor-pointer truncate w-full">
{{ content }}
</label>
{{ content }}
</slot>
<div
v-if="editing"
Expand Down Expand Up @@ -55,7 +53,11 @@ const divHeight = ref(0)
const parentRef = ref(null) // Ref for parent element
const editInputRef = ref(null) // Ref for edit input element

const startEditing = () => {
const startEditing = (event) => {
if (editing.value || (event.type === 'keydown' && event.target !== parentRef.value)) {
return
}

if (parentRef.value) {
divHeight.value = parentRef.value.offsetHeight
editing.value = true
Expand Down
19 changes: 13 additions & 6 deletions client/components/open/forms/OpenCompleteForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@
:href="getFontUrl"
>

<h1
v-if="!isHideTitle"
class="mb-4 px-2"
:class="{'mt-4':isEmbedPopup}"
v-text="form.title"
/>
<template v-if="!isHideTitle">
<EditableTag
v-if="adminPreview"
v-model="form.title"
element="h1"
class="mb-2"
/>
<h1
v-else
class="mb-2 px-2"
v-text="form.title"
/>
</template>

<div v-if="isPublicFormPage && form.is_password_protected">
<p class="form-description mb-4 text-gray-700 dark:text-gray-300 px-2">
Expand Down
2 changes: 1 addition & 1 deletion client/components/open/forms/OpenForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
/>
<has-error
:form="dataForm"
field="h-captcha-response"
field-id="h-captcha-response"
/>
</div>
</template>
Expand Down
9 changes: 5 additions & 4 deletions client/components/open/forms/OpenFormField.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
>
<div
v-if="adminPreview"
class="absolute translate-y-full lg:translate-y-0 -bottom-1.5 left-1/2 -translate-x-1/2 lg:-translate-x-full lg:-left-1 lg:top-1 lg:bottom-0 hidden group-hover/nffield:block"
class="absolute translate-y-full lg:translate-y-0 -bottom-1 left-1/2 -translate-x-1/2 lg:-translate-x-full lg:-left-1 lg:top-1 lg:bottom-0 hidden group-hover/nffield:block"
>
<div
class="flex lg:flex-col bg-gray-100 dark:bg-gray-800 border rounded-md"
Expand Down Expand Up @@ -68,15 +68,15 @@
v-if="field.type === 'nf-text' && field.content"
:id="field.id"
:key="field.id"
class="nf-text w-full mb-3"
class="nf-text w-full my-1.5"
:class="[getFieldAlignClasses(field)]"
v-html="field.content"
/>
<div
v-if="field.type === 'nf-code' && field.content"
:id="field.id"
:key="field.id"
class="nf-code w-full px-2 mb-3"
class="nf-code w-full px-2 my-1.5"
v-html="field.content"
/>
<div
Expand All @@ -91,6 +91,7 @@
:key="field.id"
class="my-4 w-full px-2"
:class="[getFieldAlignClasses(field)]"
@dblclick="editFieldOptions"
>
<div
v-if="!field.image_block"
Expand All @@ -115,7 +116,7 @@
class="hidden group-hover/nffield:flex translate-x-full absolute right-0 top-0 h-full w-5 flex-col justify-center pl-1 pt-3"
>
<div
class="flex items-center bg-gray-100 dark:bg-gray-800 border rounded-md h-12 text-gray-500 dark:text-gray-400 dark:border-gray-500 cursor-grab handle"
class="flex items-center bg-gray-100 dark:bg-gray-800 border rounded-md h-12 text-gray-500 dark:text-gray-400 dark:border-gray-500 cursor-grab handle min-h-[40px]"
>
<Icon
name="clarity:drag-handle-line"
Expand Down
12 changes: 10 additions & 2 deletions client/components/open/forms/components/FormEditor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@
id="form-editor"
class="relative flex w-full flex-col grow max-h-screen"
>
<!-- Loading overlay -->
<div
v-if="updateFormLoading"
class="absolute inset-0 bg-white bg-opacity-70 z-50 flex items-center justify-center"
>
<loader class="h-6 w-6 text-blue-500" />
</div>
<div
class="border-b bg-white md:hidden fixed inset-0 w-full z-50 flex flex-col items-center justify-center"
>
Expand Down Expand Up @@ -175,6 +182,7 @@ export default {

mounted() {
this.$emit("mounted")
this.workingFormStore.activeTab = 0
useAmplitude().logEvent('form_editor_viewed')
this.appStore.hideNavbar()
if (!this.isEdit) {
Expand Down Expand Up @@ -292,6 +300,8 @@ export default {
slug: this.createdFormSlug,
new_form: response.users_first_form,
},
}).then(() => {
this.updateFormLoading = false
})
})
.catch((error) => {
Expand All @@ -304,8 +314,6 @@ export default {
)
captureException(error)
}
})
.finally(() => {
this.updateFormLoading = false
})
},
Expand Down
Loading
Loading