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

enhance: header for form builder UI redesign #1488

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

enhance: header for form builder UI redesign #1488

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
63151b3
initial header style
sapayth Oct 7, 2024
787c082
preview and save buttons
sapayth Oct 7, 2024
a7c5fe7
dropdown effect enhanced
sapayth Oct 9, 2024
d008e8a
shortcode copy icon enhanced
sapayth Oct 9, 2024
c96afbf
backward compatibility added for old wpuf pro
sapayth Oct 10, 2024
065b1fa
update copy shortcode icon
sapayth Oct 10, 2024
6607241
Merge branch 'develop' into enhance/header_for_form_builder_ui_redesign
sapayth Oct 21, 2024
ab3ce2c
type and image fixing
sapayth Oct 22, 2024
3eba5a0
Merge remote-tracking branch 'upstream/develop' into enhance/header_f…
sapayth Oct 31, 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
46 changes: 44 additions & 2 deletions Gruntfile.js
View file
Open in desktop
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.

🛠️ Refactor suggestion

Inconsistent use of const instead of var

The codebase primarily uses var for variable declarations, but the new code introduces const for tailwindFileMap. For consistency, consider using var, or update the entire file to use ES6 declarations (let and const) where appropriate.


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 241
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential issue with dynamic parameters in the shell:tailwind command

The shell:tailwind task defines command as a function accepting input and output parameters, and you're attempting to pass these parameters via grunt.task.run(\shell:tailwind:${inputFile}:${outputFile}`). However, grunt-shell` may not support passing arguments this way, as additional colons are interpreted as sub-targets, not function parameters.

Consider modifying the task to accept arguments differently or restructure the shell command to use Grunt templates, like so:

tailwind: {
  command: 'npx tailwindcss -i <%= inputFile %> -o <%= outputFile %>'
}

And set grunt.config.set('inputFile', inputFile); and grunt.config.set('outputFile', outputFile); before running the task.

}
});
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' );
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

grunt-postcss is loaded but not configured

You have loaded grunt-postcss using grunt.loadNpmTasks('grunt-postcss');, but there is no corresponding postcss task configured in grunt.initConfig. This could lead to confusion or unintended behavior.


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.

⚠️ Potential issue

Asynchronous tailwind task may not wait for shell commands to complete

In the tailwind task, you're using this.async() and calling done(), but since grunt.task.run queues tasks to run after the current task completes, the shell:tailwind tasks might not finish before done() is called. This can lead to race conditions where the Tailwind CSS files aren't fully processed before other tasks depend on them.

Consider restructuring the tailwind task to ensure it waits for the shell commands to complete. One approach is to use grunt.task.run without this.async(), as follows:

grunt.registerTask('tailwind', function() {
  // 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)) {
      // Set the input and output files
      grunt.config.set('inputFile', inputFile);
      grunt.config.set('outputFile', outputFile);

      // Run the tailwind shell command
      grunt.task.run('shell:tailwind');
    }
  });
});

And modify the shell:tailwind task to use Grunt template variables:

tailwind: {
  command: 'npx tailwindcss -i <%= inputFile %> -o <%= outputFile %>'
}

Would you like assistance in implementing this refactoring to ensure the Tailwind CSS compilation runs smoothly?

};
44 changes: 36 additions & 8 deletions admin/form-builder/assets/js/components/form-checkbox_field/template.php
View file
Open in desktop
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>

<p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
Comment on lines +2 to +43
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring to reduce code duplication.

The template effectively supports both inline and vertical layouts with consistent use of Tailwind CSS classes. However, there's an opportunity to reduce code duplication between the two layout options.

Consider extracting the common checkbox rendering logic into a separate component or method. This could be achieved by creating a new Vue component for the checkbox item or using a method to render the checkbox. Here's a conceptual example:

<template>
  <div :class="containerClass">
    <checkbox-item
      v-for="(label, val) in field.options"
      :key="val"
      :value="val"
      :label="label"
      :checked="is_selected(val)"
      :class="checkboxClass"
    />
  </div>
  <p v-if="field.help" class="wpuf-mt-2 wpuf-text-sm wpuf-text-gray-500" v-html="field.help"></p>
</template>

<script>
export default {
  // ... other component logic ...
  computed: {
    containerClass() {
      return this.field.inline === 'yes'
        ? 'wpuf-flex'
        : 'wpuf-space-y-5';
    },
    checkboxClass() {
      return this.field.inline === 'yes'
        ? 'wpuf-mr-4'
        : '';
    }
  }
}
</script>

This approach would centralize the checkbox rendering logic, making it easier to maintain and update in the future.

</div>
7 changes: 3 additions & 4 deletions admin/form-builder/assets/js/components/form-dropdown_field/template.php
View file
Open in desktop
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>
6 changes: 4 additions & 2 deletions admin/form-builder/assets/js/components/form-post_title/template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="wpuf-fields">
<input
disabled
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>
31 changes: 18 additions & 13 deletions admin/form-builder/assets/js/components/form-radio_field/template.php
View file
Open in desktop
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">
<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">
<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>
sapayth marked this conversation as resolved.
Show resolved Hide resolved
</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>
Comment on lines 1 to +20
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Country Dropdown Not Addressed in This PR

The changes in admin/form-builder/assets/js/components/form-radio_field/template.php focus solely on the radio button component and do not address the country dropdown functionality outlined in issue #638. The broader search indicates that the country dropdown logic exists in multiple files, none of which have been modified in this PR.

To fully meet the PR objectives:

  1. Update the country dropdown component to limit the selection to North American countries as requested.
  2. Add a configuration option in the form builder to allow admins to specify available countries in the dropdown.

These additions are necessary to enhance the billing address section's usability and align with user feedback.

🔗 Analysis chain

Summary of changes and alignment with PR objectives

The changes to this file align well with the PR objective of enhancing the form builder UI. The integration of Tailwind CSS classes improves the styling and layout of the radio button fields, contributing to a more visually appealing and consistent design.

However, the changes don't directly address the specific user request mentioned in issue #638 regarding the country dropdown in the billing address section. This file focuses on the radio button component, which is a separate concern.

To fully meet the PR objectives:

  1. Ensure that the country dropdown component (which is likely in a different file) is updated to allow for a limited selection of countries, focusing on North America as requested.
  2. Consider adding a configuration option to the form builder that allows admins to specify which countries should be available in the dropdown.

These additional changes would more directly address the user feedback and improve the usability of the billing address section.

To verify if the country dropdown has been addressed in other files, you can run the following script:

This will help identify if and where the country dropdown functionality has been modified to meet the user's request.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for files that might contain country dropdown logic
rg -i -t php -t js "country.*dropdown" --glob '!vendor/*'

Length of output: 59

Script:

#!/bin/bash
# Broad search for 'country' related terms in PHP and JS files
rg -i -t php -t js "country" --glob '!vendor/*'

Length of output: 23363

</div>
15 changes: 10 additions & 5 deletions admin/form-builder/assets/js/components/form-taxonomy/template.php
View file
Open in desktop
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>

<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"
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>
6 changes: 4 additions & 2 deletions admin/form-builder/assets/js/components/form-text_field/template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="wpuf-fields">
<input
disabled
sapayth marked this conversation as resolved.
Show resolved Hide resolved
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>
sapayth marked this conversation as resolved.
Show resolved Hide resolved
</div>
14 changes: 9 additions & 5 deletions admin/form-builder/assets/js/components/form-textarea_field/template.php
View file
Open in desktop
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>

sapayth marked this conversation as resolved.
Show resolved Hide resolved

<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>
18 changes: 11 additions & 7 deletions admin/form-builder/assets/js/form-builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,8 @@
is_form_saved: false,
is_form_switcher: false,
post_title_editing: false,
isDirty: false
isDirty: false,
shortcodeCopied: false,
},

computed: {
Expand Down Expand Up @@ -491,13 +492,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 Down Expand Up @@ -899,11 +903,11 @@

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

$("#collapse-menu").click(function () {
resizeBuilderContainer();
});
// resizeBuilderContainer();
//
// $("#collapse-menu").click(function () {
// resizeBuilderContainer();
// });

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