-
Notifications
You must be signed in to change notification settings - Fork 147
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: form builder field Cloudflare Turnstile #1496
Changes from all commits
b9e0014
6b01a8c
0dbd738
89e7c24
6aa3b3f
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 |
---|---|---|
|
@@ -140,6 +140,7 @@ function admin_init() { | |
'max' => isset( $option['max'] ) ? $option['max'] : '', | ||
'step' => isset( $option['step'] ) ? $option['step'] : '', | ||
'is_pro_preview' => ! empty( $option['is_pro_preview'] ) ? $option['is_pro_preview'] : false, | ||
'depends_on' => ! empty( $option['depends_on'] ) ? $option['depends_on'] : '', | ||
); | ||
|
||
add_settings_field( $section . '[' . $option['name'] . ']', $option['label'], (isset($option['callback']) ? $option['callback'] : array($this, 'callback_' . $type )), $section, $section, $args ); | ||
|
@@ -173,14 +174,17 @@ public function get_field_description( $args ) { | |
* @param array $args settings field args | ||
*/ | ||
function callback_text( $args ) { | ||
|
||
$value = esc_attr( $this->get_option( $args['id'], $args['section'], $args['std'] ) ); | ||
$size = isset( $args['size'] ) && !is_null( $args['size'] ) ? $args['size'] : 'regular'; | ||
$type = isset( $args['type'] ) ? $args['type'] : 'text'; | ||
$placeholder = empty( $args['placeholder'] ) ? '' : ' placeholder="' . $args['placeholder'] . '"'; | ||
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : ''; | ||
$depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : ''; | ||
|
||
$html = sprintf( '<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s/>', $type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled ); | ||
$html = sprintf( | ||
'<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" />', | ||
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on | ||
); | ||
Comment on lines
+182
to
+187
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. Escape the 'depends_on' attribute to prevent XSS vulnerabilities The Apply this diff to escape the $depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : '';
$html = sprintf(
- '<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" />',
+ '<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" />',
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on
); Updated code: $depends_on = ! empty( $args['depends_on'] ) ? esc_attr( $args['depends_on'] ) : '';
$html = sprintf(
'<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" />',
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on
); |
||
$html .= $this->get_field_description( $args ); | ||
|
||
if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) { | ||
|
@@ -460,6 +464,38 @@ function callback_color( $args ) { | |
echo $html; | ||
} | ||
|
||
/** | ||
* Displays a toggle field for a settings field | ||
* | ||
* @param array $args settings field args | ||
*/ | ||
public function callback_toggle( $args ) { | ||
$value = esc_attr( $this->get_option( $args['id'], $args['section'], $args['std'] ) ); | ||
$disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : ''; | ||
$name = $args['section'] . '[' . $args['id'] . ']'; | ||
?> | ||
<fieldset> | ||
<label for="<?php echo 'wpuf-' . $name; ?>" class="wpuf-toggle-switch"> | ||
<input | ||
type="hidden" | ||
name="<?php echo $name; ?>" | ||
value="off" /> | ||
<input | ||
type="checkbox" | ||
<?php echo $value === 'on' ? 'checked' : ''; ?> | ||
<?php echo $disabled ? 'disabled' : ''; ?> | ||
id="<?php echo 'wpuf-' . $name; ?>" | ||
name="<?php echo $name; ?>" | ||
class="wpuf-toggle-module checkbox" | ||
value="on"> | ||
<span class="slider round"></span> | ||
</label> | ||
</fieldset> | ||
|
||
<?php echo $args['desc']; ?> | ||
<?php | ||
} | ||
Comment on lines
+472
to
+497
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. Sanitize the 'toggle' field input before saving In the |
||
|
||
/** | ||
* Sanitize callback for Settings API | ||
* | ||
|
@@ -673,6 +709,44 @@ function(){ | |
|
||
// disable the pro preview checkboxes | ||
$('span.pro-icon-title').siblings('input[type="checkbox"]').prop('disabled', true); | ||
|
||
var fields = $('table.form-table input, table.form-table select, table.form-table textarea'); | ||
|
||
// iterate over each field and check if it depends on another field | ||
fields.each(function() { | ||
var $this = $(this); | ||
var data_depends_on = $this.data('depends-on'); | ||
|
||
if (!data_depends_on) { | ||
return; | ||
} | ||
|
||
var $depends_on = $("input[id*='"+ data_depends_on +"']"); | ||
var $depends_on_type = $depends_on.attr('type'); | ||
|
||
if ($depends_on_type === 'checkbox') { | ||
if ($depends_on.is(':checked')) { | ||
$this.closest('tr').show(); | ||
} else { | ||
$this.closest('tr').hide(); | ||
} | ||
$depends_on.on('change', function() { | ||
if ($(this).is(':checked')) { | ||
$this.closest('tr').show(); | ||
} else { | ||
$this.closest('tr').hide(); | ||
} | ||
}); | ||
} else { | ||
$depends_on.on('keyup change', function() { | ||
if ($(this).val() === $this.data('depends-on-value')) { | ||
$this.closest('tr').show(); | ||
} else { | ||
$this.closest('tr').hide(); | ||
} | ||
}); | ||
} | ||
Comment on lines
+741
to
+748
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. 💡 Codebase verification The The code shows that while
sprintf(
'<input type="%1$s" class="%2$s-text" id="%3$s[%4$s]" name="%3$s[%4$s]" value="%5$s"%6$s %7$s data-depends-on="%8$s" data-depends-on-value="%9$s" />',
$type, $size, $args['section'], $args['id'], $value, $placeholder, $disabled, $depends_on, $depends_on_value
) 🔗 Analysis chainVerify the existence of 'data-depends-on-value' attribute in HTML The JavaScript code references Run the following script to search for the usage of 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Search the codebase for occurrences of 'data-depends-on-value'
# Test: Expect to find all HTML elements that use 'data-depends-on-value' attribute.
rg 'data-depends-on-value'
Length of output: 26 Script: #!/bin/bash
# Let's search for where this data attribute might be set in PHP code
# Search for variations of setting data-depends-on-value
rg -i "data.*depends.*value" -A 3
# Also search for the method that might be setting up these dependencies
ast-grep --pattern 'function $_ ($$$) {
$$$
depends
$$$
}'
Length of output: 21224 Script: #!/bin/bash
# Let's search for where this dependency functionality is set up
rg -i "depends.*on" -B 3 -A 3 Lib/WeDevs_Settings_API.php
# Also search for any data attribute setting in the PHP code
ast-grep --pattern 'data($$$)' Lib/WeDevs_Settings_API.php
Length of output: 2970 |
||
}); | ||
Comment on lines
+712
to
+749
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 Optimize the JavaScript for field dependencies The JavaScript code handling field dependencies can be optimized for improved performance and readability. Repeated DOM queries within loops may affect efficiency. Consider the following refactoring suggestions:
Example refactored code snippet: var $fields = $('table.form-table input, table.form-table select, table.form-table textarea');
// iterate over each field and check if it depends on another field
$fields.each(function() {
var $this = $(this);
var dependsOn = $this.data('depends-on');
if (!dependsOn) {
return;
}
var $dependsOn = $("input[id*='" + dependsOn + "']");
var dependsOnType = $dependsOn.attr('type');
var toggleFieldVisibility = function() {
if (dependsOnType === 'checkbox') {
$this.closest('tr').toggle($dependsOn.is(':checked'));
} else {
$this.closest('tr').toggle($dependsOn.val() === $this.data('depends-on-value'));
}
};
// Initial check
toggleFieldVisibility();
// Event binding
var events = (dependsOnType === 'checkbox') ? 'change' : 'keyup change';
$dependsOn.on(events, toggleFieldVisibility);
}); |
||
}); | ||
</script> | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Field template: Cloudflare Turnstile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Vue.component('form-cloudflare_turnstile', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
template: '#tmpl-wpuf-form-cloudflare_turnstile', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mixins: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
wpuf_mixins.form_field_mixin | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
computed: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
has_turnstile_api_keys: function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return wpuf_form_builder.turnstile_site && wpuf_form_builder.turnstile_secret; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+12
to
+14
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. Add defensive checks for global object access The code assumes Apply this diff to add defensive checks: has_turnstile_api_keys: function () {
- return wpuf_form_builder.turnstile_site && wpuf_form_builder.turnstile_secret;
+ return typeof wpuf_form_builder !== 'undefined'
+ && wpuf_form_builder.turnstile_site
+ && wpuf_form_builder.turnstile_secret;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
no_api_keys_msg: function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return wpuf_form_builder.field_settings.turnstile.validator.msg; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+16
to
+18
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. Add null check for field settings access Similar to the previous issue, add defensive checks for accessing nested properties. Apply this diff to prevent potential undefined errors: no_api_keys_msg: function () {
- return wpuf_form_builder.field_settings.turnstile.validator.msg;
+ return wpuf_form_builder?.field_settings?.turnstile?.validator?.msg || 'API keys are required';
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
turnstile_image: function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var base_url = wpuf_form_builder.asset_url + '/images/cloudflare-placeholder-'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (this.field.turnstile_theme === 'dark') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
base_url += 'dark'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
base_url += 'light'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (this.field.turnstile_size === 'compact') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
base_url += '-compact'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return base_url + '.png'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+20
to
+34
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. Enhance input validation for theme and size properties The function should validate the theme and size values before using them in URL construction. Also, consider using a constant for the base URL. Apply this diff to improve the implementation: + const VALID_THEMES = ['dark', 'light'];
+ const VALID_SIZES = ['normal', 'compact'];
+ const BASE_PLACEHOLDER_PATH = '/images/cloudflare-placeholder-';
turnstile_image: function () {
- var base_url = wpuf_form_builder.asset_url + '/images/cloudflare-placeholder-';
+ if (typeof wpuf_form_builder === 'undefined' || !wpuf_form_builder.asset_url) {
+ console.error('Asset URL is not defined');
+ return '';
+ }
+
+ const theme = VALID_THEMES.includes(this.field.turnstile_theme)
+ ? this.field.turnstile_theme
+ : 'light';
+
+ const size = VALID_SIZES.includes(this.field.turnstile_size)
+ ? this.field.turnstile_size
+ : 'normal';
+
+ let imagePath = BASE_PLACEHOLDER_PATH + theme;
+ if (size === 'compact') {
+ imagePath += '-compact';
+ }
+
+ return wpuf_form_builder.asset_url + imagePath + '.png';
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<div class="wpuf-fields"> | ||
<template v-if="!has_turnstile_api_keys"> | ||
<p v-html="no_api_keys_msg"></p> | ||
</template> | ||
|
||
<template v-else> | ||
<img | ||
class="wpuf-turnstile-placeholder" | ||
:src="turnstile_image" | ||
alt=""> | ||
</template> | ||
Comment on lines
+7
to
+11
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 Enhance image accessibility and loading behavior. The image element could be improved for better accessibility and user experience. <img
class="wpuf-turnstile-placeholder"
:src="turnstile_image"
- alt="">
+ :alt="'Cloudflare Turnstile verification widget'"
+ loading="lazy"
+ @error="handleImageLoadError"> Consider adding:
|
||
</div> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -44,6 +44,10 @@ Vue.mixin({ | |||||||||||||||||
return (wpuf_form_builder.recaptcha_site && wpuf_form_builder.recaptcha_secret) ? true : false; | ||||||||||||||||||
}, | ||||||||||||||||||
|
||||||||||||||||||
has_turnstile_api_keys: function () { | ||||||||||||||||||
return wpuf_form_builder.turnstile_site && wpuf_form_builder.turnstile_secret; | ||||||||||||||||||
}, | ||||||||||||||||||
Comment on lines
+47
to
+49
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 Add defensive programming for global object access. Consider adding a safety check for the has_turnstile_api_keys: function () {
- return wpuf_form_builder.turnstile_site && wpuf_form_builder.turnstile_secret;
+ return typeof wpuf_form_builder !== 'undefined'
+ && wpuf_form_builder.turnstile_site
+ && wpuf_form_builder.turnstile_secret;
}, 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
containsField: function(field_name) { | ||||||||||||||||||
var self = this, | ||||||||||||||||||
i = 0; | ||||||||||||||||||
|
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.
Add documentation for the new 'depends_on' parameter
The
'depends_on'
parameter is introduced in the$args
array without accompanying documentation. For better maintainability and clarity, please update the method's PHPDoc to include a description of this new parameter and its expected usage.