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

enhance: new header form form builder UI redesign #1497

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
46 changes: 44 additions & 2 deletions Gruntfile.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
'use strict';
module.exports = function(grunt) {
module.exports = function( grunt) {
const tailwindFileMap = {
'admin/form-builder/views/form-builder.php': 'admin/form-builder.css',
}
Comment on lines +3 to +5
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add missing Tailwind file mappings for templates using Tailwind classes

The tailwindFileMap is incomplete. The following templates use Tailwind classes but lack corresponding CSS output mappings:

  • admin/form-builder/views/form-builder-old.php
  • includes/Admin/views/shortcode-builder.php
  • includes/Admin/views/premium.php
  • includes/Admin/views/post-forms-list-table-view.php
  • includes/Admin/views/support.php

Each of these files should have a mapping to its respective CSS output file to ensure proper Tailwind processing.

🔗 Analysis chain

Consider expanding the Tailwind file mapping.

The tailwindFileMap only processes one template file, but the watch task monitors multiple PHP directories. Consider adding mappings for other form builder templates that will use Tailwind CSS.


Let me check if these PHP files actually use Tailwind classes before making a final recommendation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find potential PHP templates that might need Tailwind processing
fd -e php . admin/form-builder/views includes/Admin/views

Length of output: 504


Script:

#!/bin/bash
# Check for Tailwind class usage in PHP files
echo "=== Checking admin/form-builder/views/ ==="
rg -l 'class="[^"]*(?:text-|bg-|flex|grid|p-|m-|w-|h-)' admin/form-builder/views/

echo -e "\n=== Checking includes/Admin/views/ ==="
rg -l 'class="[^"]*(?:text-|bg-|flex|grid|p-|m-|w-|h-)' includes/Admin/views/

Length of output: 600


var formBuilderAssets = require('./admin/form-builder/assets/js/form-builder-assets.js');

var pkg = grunt.file.readJSON('package.json');
Expand Down Expand Up @@ -112,7 +116,15 @@ module.exports = function(grunt) {
tasks: [
'shell:npm_build'
]
}
},

tailwind: {
files: ['src/css/**/*.css', 'admin/form-builder/views/*.php', 'includes/Admin/views/*.php'],
tasks: ['shell:tailwind'],
options: {
spawn: false
}
},
},

// Clean up build directory
Expand Down Expand Up @@ -221,6 +233,11 @@ module.exports = function(grunt) {
shell: {
npm_build: {
command: 'npm run build',
},
tailwind: {
command: function ( input, output ) {
return `npx tailwindcss -i ${input} -o ${output}`;
}
Comment on lines +237 to +240
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to the Tailwind shell command.

The command should validate the existence of the tailwindcss binary and handle potential errors.

 shell: {
     tailwind: {
         command: function ( input, output ) {
-            return `npx tailwindcss -i ${input} -o ${output}`;
+            return `
+                if ! command -v npx &> /dev/null; then
+                    echo "Error: npx is not installed"
+                    exit 1
+                fi
+                if ! npx tailwindcss --help &> /dev/null; then
+                    echo "Error: tailwindcss is not installed"
+                    exit 1
+                fi
+                npx tailwindcss -i ${input} -o ${output}
+            `;
         }
     }
 }

Committable suggestion skipped: line range outside the PR's diff.

}
}
});
Expand All @@ -238,6 +255,7 @@ module.exports = function(grunt) {
grunt.loadNpmTasks( 'grunt-notify' );
grunt.loadNpmTasks( 'grunt-wp-readme-to-markdown' );
grunt.loadNpmTasks( 'grunt-shell' );
grunt.loadNpmTasks( 'grunt-postcss' );

grunt.registerTask( 'default', [ 'less', 'concat', 'uglify', 'i18n' ] );

Expand All @@ -248,4 +266,28 @@ module.exports = function(grunt) {
// build stuff
grunt.registerTask( 'release', [ 'less', 'concat', 'uglify', 'i18n', 'readme' ] );
grunt.registerTask( 'zip', [ 'clean', 'copy', 'compress' ] );

grunt.event.on('watch', function(action, filepath, target) {
if (target === 'tailwind') {
grunt.task.run('tailwind');
}
});

grunt.registerTask('tailwind', function() {
const done = this.async();

// Process each file mapping
Object.entries(tailwindFileMap).forEach(([phpFile, cssFile]) => {
const inputFile = `src/css/${cssFile}`;
const outputFile = `assets/css/${cssFile}`;

// Ensure the input file exists
if (grunt.file.exists(inputFile)) {
// Run the tailwind command
grunt.task.run(`shell:tailwind:${inputFile}:${outputFile}`);
}
});

done();
});
Comment on lines +276 to +292
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance the tailwind task with better error handling and progress feedback.

The task should provide better error handling and progress feedback. Also, consider handling concurrent file processing more safely.

 grunt.registerTask('tailwind', function() {
     const done = this.async();
+    let hasErrors = false;
 
     // Process each file mapping
     Object.entries(tailwindFileMap).forEach(([phpFile, cssFile]) => {
         const inputFile = `src/css/${cssFile}`;
         const outputFile = `assets/css/${cssFile}`;
 
-        // Ensure the input file exists
-        if (grunt.file.exists(inputFile)) {
-            // Run the tailwind command
-            grunt.task.run(`shell:tailwind:${inputFile}:${outputFile}`);
+        try {
+            if (!grunt.file.exists(inputFile)) {
+                grunt.log.error(`Input file not found: ${inputFile}`);
+                hasErrors = true;
+                return;
+            }
+
+            grunt.log.writeln(`Processing ${inputFile} -> ${outputFile}`);
+            grunt.task.run(`shell:tailwind:${inputFile}:${outputFile}`);
+        } catch (error) {
+            grunt.log.error(`Error processing ${inputFile}: ${error}`);
+            hasErrors = true;
         }
     });
 
-    done();
+    if (hasErrors) {
+        done(false);
+    } else {
+        done();
+    }
 });
📝 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
grunt.registerTask('tailwind', function() {
const done = this.async();
// Process each file mapping
Object.entries(tailwindFileMap).forEach(([phpFile, cssFile]) => {
const inputFile = `src/css/${cssFile}`;
const outputFile = `assets/css/${cssFile}`;
// Ensure the input file exists
if (grunt.file.exists(inputFile)) {
// Run the tailwind command
grunt.task.run(`shell:tailwind:${inputFile}:${outputFile}`);
}
});
done();
});
grunt.registerTask('tailwind', function() {
const done = this.async();
let hasErrors = false;
// Process each file mapping
Object.entries(tailwindFileMap).forEach(([phpFile, cssFile]) => {
const inputFile = `src/css/${cssFile}`;
const outputFile = `assets/css/${cssFile}`;
try {
if (!grunt.file.exists(inputFile)) {
grunt.log.error(`Input file not found: ${inputFile}`);
hasErrors = true;
return;
}
grunt.log.writeln(`Processing ${inputFile} -> ${outputFile}`);
grunt.task.run(`shell:tailwind:${inputFile}:${outputFile}`);
} catch (error) {
grunt.log.error(`Error processing ${inputFile}: ${error}`);
hasErrors = true;
}
});
if (hasErrors) {
done(false);
} else {
done();
}
});

};
Original file line number Diff line number Diff line change
@@ -1,16 +1,44 @@
<div class="wpuf-fields">
<ul :class="['wpuf-fields-list', ('yes' === field.inline) ? 'wpuf-list-inline' : '']">
<li v-if="has_options" v-for="(label, val) in field.options">
<label>
<div
v-if="field.inline !== 'yes'"
class="wpuf-space-y-5">
<div
v-if="has_options" v-for="(label, val) in field.options"
class="wpuf-relative wpuf-flex wpuf-items-start">
<div class="wpuf-items-center">
<input
type="checkbox"
:value="val"
:checked="is_selected(val)"
:class="class_names('checkbox_btns')"
> {{ label }}
</label>
</li>
</ul>
class="wpuf-h-4 wpuf-w-4 wpuf-rounded wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">
</div>
<div class="wpuf-ml-1 wpuf-text-sm">
<label class="wpuf-font-medium wpuf-text-gray-900">{{ label }}</label>
</div>
</div>
</div>

<span v-if="field.help" class="wpuf-help" v-html="field.help" />
<div
v-if="field.inline === 'yes'"
class="wpuf-flex"
>
<div
v-if="has_options" v-for="(label, val) in field.options"
class="wpuf-relative wpuf-flex wpuf-items-start wpuf-mr-4">
<div class="wpuf-items-center">
<input
type="checkbox"
:value="val"
:checked="is_selected(val)"
:class="class_names('checkbox_btns')"
class="wpuf-h-4 wpuf-w-4 wpuf-rounded wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">
</div>
<div class="wpuf-ml-1 wpuf-text-sm">
<label class="wpuf-font-medium wpuf-text-gray-900">{{ label }}</label>
</div>
</div>
</div>
Comment on lines +22 to +41
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce code duplication and improve responsiveness

The inline layout duplicates much of the block layout code. Consider refactoring to reuse the checkbox item template and improve responsive behavior.

-<div
-    v-if="field.inline === 'yes'"
-    class="wpuf-flex"
->
+<div
+    v-if="field.inline === 'yes'"
+    class="wpuf-flex wpuf-flex-wrap wpuf-gap-4"
+    role="group"
+    :aria-label="field.label">
     <div
         v-if="has_options" v-for="(label, val) in field.options"
-        class="wpuf-relative wpuf-flex wpuf-items-start wpuf-mr-4">
+        class="wpuf-relative wpuf-flex wpuf-items-start">

Consider extracting the checkbox item into a separate component to reduce duplication:

<!-- checkbox-item.vue -->
<template>
  <div class="wpuf-relative wpuf-flex wpuf-items-start">
    <div class="wpuf-items-center">
      <input
        type="checkbox"
        :value="value"
        :checked="checked"
        :class="className"
        :aria-disabled="disabled"
        class="wpuf-h-4 wpuf-w-4 wpuf-rounded wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">
    </div>
    <div class="wpuf-ml-1 wpuf-text-sm">
      <label class="wpuf-font-medium wpuf-text-gray-900">{{ label }}</label>
    </div>
  </div>
</template>


<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
<div class="wpuf-fields">
<select
disabled
:class="class_names('select_lbl')"
>
class="wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6">
<option v-if="field.first" value="">{{ field.first }}</option>

<option
v-if="has_options"
v-for="(label, val) in field.options"
:value="label"
:selected="is_selected(label)"
>{{ label }}</option>
</select>

<span v-if="field.help" class="wpuf-help" v-html="field.help"> </span>
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="wpuf-fields">
<input
disabled
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove the disabled attribute from the post title input.

The disabled attribute prevents user input, which contradicts the purpose of a post title field in a form builder. Users need to be able to enter and edit post titles.

Apply this diff to fix the issue:

    <input
-        disabled
        type="text"
        :placeholder="field.placeholder"
📝 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
disabled
type="text"
:placeholder="field.placeholder"

type="text"
:class="class_names('textfield')"
:placeholder="field.placeholder"
:value="field.default"
:size="field.size"
:class="class_names('textfield')"
class="wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
>
<span v-if="field.help" class="wpuf-help" v-html="field.help" />
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
<div class="wpuf-fields">
<ul :class="['wpuf-fields-list', ('yes' === field.inline) ? 'wpuf-list-inline' : '']">
<li v-if="has_options" v-for="(label, val) in field.options">
<label>
<input
type="radio"
:value="val"
:checked="is_selected(val)"
:class="class_names('radio_btns')"
> {{ label }}
</label>
</li>
</ul>
<div
v-if="field.inline !== 'yes'"
class="wpuf-space-y-6">
Comment on lines +2 to +4
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing inline radio button implementation

The code shows a condition for non-inline radio buttons (field.inline !== 'yes') but lacks the corresponding implementation for when field.inline === 'yes'. This is inconsistent with the checkbox field implementation which properly handles both cases. The inline radio layout should be implemented to maintain consistency with the checkbox field pattern.

  • Missing implementation for v-if="field.inline === 'yes'" case in admin/form-builder/assets/js/components/form-radio_field/template.php
  • Should follow similar pattern as checkbox field with wpuf-flex container and wpuf-mr-4 spacing between options
🔗 Analysis chain

Verify the implementation of inline radio buttons.

The condition field.inline !== 'yes' suggests there should be an alternate layout for inline radio buttons, but the implementation for the inline case is not visible in this code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any inline radio button implementations
rg -l "field.inline.*yes" --type php

Length of output: 221


Script:

#!/bin/bash
# Check the implementation details of inline radio buttons in the template files
rg -A 10 "field\.inline.*yes" --type php

Length of output: 6349


Script:

#!/bin/bash
# Let's check for the inline radio implementation in the same file
rg -C 15 "field\.inline === 'yes'" admin/form-builder/assets/js/components/form-radio_field/template.php

Length of output: 112

<div
v-if="has_options" v-for="(label, val) in field.options"
class="wpuf-flex wpuf-items-center">
<input
type="radio"
class="wpuf-h-4 wpuf-w-4 wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">
Comment on lines +8 to +10
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add required Vue.js directives for proper functionality.

The radio input is missing essential Vue.js directives:

  1. v-model for two-way data binding
  2. :key attribute for v-for iteration
  3. name attribute to group radio buttons
 <input
     type="radio"
+    v-model="field.value"
+    :name="field.name"
+    :key="val"
     class="wpuf-h-4 wpuf-w-4 wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">

Committable suggestion skipped: line range outside the PR's diff.

<label
disabled
:value="val"
:checked="is_selected(val)"
:class="class_names('radio_btns')"
class="wpuf-ml-3 wpuf-block wpuf-text-sm wpuf-font-medium wpuf-leading-6 wpuf-text-gray-900">{{ label }}</label>
Comment on lines +11 to +16
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove disabled attribute from label and improve accessibility.

  1. The disabled attribute on labels is incorrect and may affect accessibility
  2. Missing aria-label for screen readers
  3. The label should be associated with its input using for attribute
-<label
-    disabled
-    :value="val"
-    :checked="is_selected(val)"
-    :class="class_names('radio_btns')"
-    class="wpuf-ml-3 wpuf-block wpuf-text-sm wpuf-font-medium wpuf-leading-6 wpuf-text-gray-900">{{ label }}</label>
+<label
+    :for="field.name + '_' + val"
+    :class="class_names('radio_btns')"
+    class="wpuf-ml-3 wpuf-block wpuf-text-sm wpuf-font-medium wpuf-leading-6 wpuf-text-gray-900">{{ label }}</label>

Also update the input accordingly:

 <input
     type="radio"
+    :id="field.name + '_' + val"
     class="wpuf-h-4 wpuf-w-4 wpuf-border-gray-300 wpuf-text-indigo-600 focus:wpuf-ring-indigo-600">

Committable suggestion skipped: line range outside the PR's diff.

</div>
</div>

<span v-if="field.help" class="wpuf-help" v-html="field.help"/>
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sanitize help text to prevent XSS attacks.

Using v-html with potentially unsafe content can lead to XSS vulnerabilities. Consider either:

  1. Using v-text if HTML rendering is not required
  2. Implementing proper sanitization before rendering
-<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
+<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-text="field.help"></p>

If HTML rendering is necessary, implement sanitization:

+computed: {
+    sanitizedHelp() {
+        return this.sanitizeHTML(this.field.help);
+    }
+},

Then use:

-<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
+<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="sanitizedHelp"></p>

Committable suggestion skipped: line range outside the PR's diff.

</div>
15 changes: 10 additions & 5 deletions admin/form-builder/assets/js/components/form-taxonomy/template.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<div class="wpuf-fields">
<select
v-if="'select' === field.type"
disabled
:class="field.name"
v-html ="get_term_dropdown_options()"
/>
class="wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
v-html ="get_term_dropdown_options()">
</select>
Comment on lines +4 to +8
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Several improvements needed for the select field implementation.

  1. The disabled attribute should be dynamic to allow conditional enabling/disabling:
  2. Class bindings can be consolidated
  3. Using v-html with get_term_dropdown_options() poses potential XSS risks

Consider applying these changes:

 <select
     v-if="'select' === field.type"
-    disabled
-    :class="field.name"
-    class="wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
-    v-html="get_term_dropdown_options()">
+    :disabled="isFieldDisabled"
+    :class="['wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6', field.name]">
+    <option v-for="option in getTermOptions()" 
+            :key="option.value" 
+            :value="option.value">
+        {{ option.label }}
+    </option>
 </select>
📝 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
disabled
:class="field.name"
v-html ="get_term_dropdown_options()"
/>
class="wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
v-html ="get_term_dropdown_options()">
</select>
:disabled="isFieldDisabled"
:class="['wpuf-block wpuf-w-full !wpuf-max-w-full wpuf-rounded-md wpuf-border-0 wpuf-text-gray-900 wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 focus:wpuf-ring-2 focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6', field.name]">
<option v-for="option in getTermOptions()"
:key="option.value"
:value="option.value">
{{ option.label }}
</option>
</select>


<div v-if="'ajax' === field.type" class="category-wrap">
<div>
Expand Down Expand Up @@ -34,11 +36,14 @@

<input
v-if="'text' === field.type"
class="textfield"
type="text"
disabled="disabled"
:class="class_names('textfield')"
class="wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
:placeholder="field.placeholder"
:size="field.size"
Comment on lines +40 to +44
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize the text field implementation.

  1. The disabled attribute has a redundant value
  2. Class bindings can be consolidated
  3. Empty value attribute can be removed

Apply these improvements:

 <input
     v-if="'text' === field.type"
     type="text"
-    disabled="disabled"
-    :class="class_names('textfield')"
-    class="wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
+    disabled
+    :class="[
+      class_names('textfield'),
+      'wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6'
+    ]"
     :placeholder="field.placeholder"
-    :size="field.size"
-    value=""
+    :size="field.size"
     autocomplete="off"
 >

Committable suggestion skipped: line range outside the PR's diff.

value=""
size="40"
autocomplete="off"
>
<span v-if="field.help" class="wpuf-help" v-html="field.help" />
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="wpuf-fields">
<input
disabled
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove the disabled attribute.

The disabled attribute prevents user interaction with the text field, which appears to be unintentional and contradicts the form's purpose. This could be blocking users from entering data.

Apply this diff to fix the issue:

    <input
-        disabled
        type="text"
📝 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
disabled
type="text"

type="text"
:class="class_names('textfield')"
:placeholder="field.placeholder"
:value="field.default"
:size="field.size"
:class="class_names('textfield')"
class="wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6"
>
<span v-if="field.help" class="wpuf-help" v-html="field.help" />
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
<div class="wpuf-fields">
<textarea
v-if="'no' === field.rich"
:class="class_names('textareafield')"
:placeholder="field.placeholder"
:deault="field.default"
:default="field.default"
:rows="field.rows"
:cols="field.cols"
>{{ field.default }}</textarea>
:class="class_names('textareafield')"
class="wpuf-block wpuf-w-full wpuf-rounded-md wpuf-border-0 wpuf-py-1.5 wpuf-text-gray-900 wpuf-shadow-sm wpuf-ring-1 wpuf-ring-inset wpuf-ring-gray-300 placeholder:wpuf-text-gray-400 focus:wpuf-ring-2 focus:wpuf-ring-inset focus:wpuf-ring-indigo-600 sm:wpuf-text-sm sm:wpuf-leading-6">{{ field.default }}</textarea>


<text-editor v-if="'no' !== field.rich" :default_text="field.default" :rich="field.rich"></text-editor>
<text-editor
v-if="'no' !== field.rich"
:default_text="field.default"
:rich="field.rich"></text-editor>

<span v-if="field.help" class="wpuf-help" v-html="field.help" />
<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</div>
50 changes: 14 additions & 36 deletions admin/form-builder/assets/js/form-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,10 @@
is_form_saved: false,
is_form_switcher: false,
post_title_editing: false,
isDirty: false
isDirty: false,
enableMultistep: false,
shortcodeCopied: false,
active_tab: 'form-editor',
},

computed: {
Expand Down Expand Up @@ -448,7 +451,7 @@
});

return meta_key.map(function(name) { return '{' + name +'}' }).join( );
}
},
},

watch: {
Expand All @@ -471,18 +474,6 @@
},

mounted: function () {
// primary nav tabs and their contents
this.bind_tab_on_click($('#wpuf-form-builder > fieldset > .nav-tab-wrapper > a'), '#wpuf-form-builder');

// secondary settings tabs and their contents
var settings_tabs = $('#wpuf-form-builder-settings .nav-tab'),
settings_tab_contents = $('#wpuf-form-builder-settings .tab-contents .group');

settings_tabs.first().addClass('nav-tab-active');
settings_tab_contents.first().addClass('active');

this.bind_tab_on_click(settings_tabs, '#wpuf-form-builder-settings');

var clipboard = new window.Clipboard('.form-id');
$(".form-id").tooltip();

Expand All @@ -491,13 +482,16 @@
clipboard.on('success', function(e) {
// Show copied tooltip
$(e.trigger)
.attr('data-original-title', 'Copied!')
.attr('data-original-title', 'Shortcode copied!')
.tooltip('show');

self.shortcodeCopied = true;

// Reset the copied tooltip
setTimeout(function() {
$(e.trigger).tooltip('hide')
.attr('data-original-title', self.i18n.copy_shortcode);
self.shortcodeCopied = false;
}, 1000);

e.clearSelection();
Expand All @@ -511,22 +505,6 @@
},

methods: {
// tabs and their contents
bind_tab_on_click: function (tabs, scope) {
tabs.on('click', function (e) {
e.preventDefault();

var button = $(this),
tab_contents = $(scope + ' > fieldset > .tab-contents'),
group_id = button.attr('href');

button.addClass('nav-tab-active').siblings('.nav-tab-active').removeClass('nav-tab-active');

tab_contents.children().removeClass('active');
$(group_id).addClass('active');
});
},

// switch form
switch_form: function () {
this.is_form_switcher = (this.is_form_switcher) ? false : true;
Expand Down Expand Up @@ -899,11 +877,11 @@

// on DOM ready
$(function() {
resizeBuilderContainer();

$("#collapse-menu").click(function () {
resizeBuilderContainer();
});
// resizeBuilderContainer();
//
// $("#collapse-menu").click(function () {
// resizeBuilderContainer();
// });
Comment on lines +880 to +884
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove commented code

The resize functionality is commented out but the implementation is still present below. This creates confusion about whether this is temporary or should be removed entirely.

Either:

  1. Remove the commented code and its implementation if it's no longer needed
  2. Re-enable it if the functionality is still required
  3. Add a TODO comment explaining why it's temporarily disabled
-        // resizeBuilderContainer();
-        //
-        // $("#collapse-menu").click(function () {
-        //     resizeBuilderContainer();
-        // });
📝 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
// resizeBuilderContainer();
//
// $("#collapse-menu").click(function () {
// resizeBuilderContainer();
// });


function resizeBuilderContainer() {
if ($(document.body).hasClass('folded')) {
Expand Down
Loading
Loading