GDPR Update#2224
Conversation
WalkthroughThis update integrates GDPR compliance features into the form management system. It adds new settings for enabling GDPR and disabling cookies, introduces helper classes and a dedicated field type for GDPR fields, and creates corresponding admin and front-end UI components. Additionally, adjustments were made in field mapping, CSS styling, and JavaScript functionality to support conditional checkbox behavior and user interface enhancements. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant FE as Front-End Renderer
participant Settings as FrmSettings
participant GDPRHelper as FrmFieldGdprHelper
participant GDPRField as FrmFieldGdpr
User->>FE: Load form page
FE->>Settings: get_settings()
Settings-->>FE: Return settings (enable_gdpr, no_gdpr_cookies)
FE->>GDPRHelper: hide_gdpr_field()
GDPRHelper-->>FE: Return visibility status
FE->>GDPRField: get_new_field_defaults (if GDPR enabled)
GDPRField-->>FE: Render GDPR field if applicable
sequenceDiagram
participant User as User
participant JS as FormidableAdminJS (hideShowItem)
participant Checkbox as Target Checkboxes
User->>JS: Click on element with data-frmuncheck
JS->>JS: Read data-frmuncheck attribute
JS->>Checkbox: Loop through and uncheck specified checkboxes
Checkbox-->>JS: Acknowledge uncheck
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
classes/helpers/FrmAppHelper.php (1)
4414-4424: Update the @SInCE version number.The @SInCE tag uses a placeholder "x.x" which should be replaced with the actual version number for this release.
- * @since x.x + * @since 6.18LGTM! Clean implementation of GDPR cookie check.
The implementation is clean, focused and integrates well with the existing settings system. This provides a good API for checking GDPR cookie settings which is important for EU compliance.
classes/views/frm-settings/general.php (2)
77-82: Consider adding warning for cookie disabling.When disabling GDPR cookies, users should be informed about potential impacts on form functionality.
<p <?php FrmAppHelper::array_to_html_params( $gdpr_options_wrapper_params, true ); ?>> <label> <input type="checkbox" name="frm_no_gdpr_cookies" id="frm_no_gdpr_cookies" value="1" <?php checked( $frm_settings->no_gdpr_cookies, 1 ); ?> /> <?php esc_html_e( 'Disable user tracking cookies. This will disable the user tracking feature as well as the ability to limit form entries to one per user by cookie.', 'formidable' ); ?> </label> + <p class="frm_description"> + <?php esc_html_e( 'Warning: Disabling cookies may affect form submission limits and user experience. Please review your form settings after making this change.', 'formidable' ); ?> + </p> </p>
84-89: Consider adding data retention policy field.For comprehensive GDPR compliance, consider adding a field to specify the data retention period for stored IPs.
This would help users comply with GDPR's data minimization principle by automatically purging old IP addresses after the specified retention period.
classes/helpers/FrmFieldGdprHelper.php (2)
5-5: Update version placeholder.The version placeholder "x.x" should be replaced with the actual version number.
-@since x.x +@since 1.0.0
40-46: Add error handling for settings retrieval.The
get_settings()call should include error handling to gracefully handle cases where settings are not available.public static function load_hooks() { - $settings = FrmAppHelper::get_settings(); - if ( $settings->enable_gdpr ) { + try { + $settings = FrmAppHelper::get_settings(); + if ( $settings && $settings->enable_gdpr ) { + add_filter( 'frm_available_fields', array( __CLASS__, 'add_gdpr_field' ), 10, 1 ); + add_filter( 'frm_get_field_type_class', array( __CLASS__, 'init_gdpr_field_class' ), 10, 2 ); + } + } catch ( Exception $e ) { + error_log( 'Failed to load GDPR settings: ' . $e->getMessage() ); + } - add_filter( 'frm_available_fields', array( __CLASS__, 'add_gdpr_field' ), 10, 1 ); - add_filter( 'frm_get_field_type_class', array( __CLASS__, 'init_gdpr_field_class' ), 10, 2 ); - } }classes/models/fields/FrmFieldGdpr.php (3)
27-27: Consider making the view path configurable.The view path is hardcoded, which could make it difficult to override or customize in different environments.
-const VIEW_PATH = '/classes/views/frm-fields/front-end/gdpr/gdpr-field.php'; +const VIEW_PATH = FORMIDABLE_VIEWS_PATH . '/frm-fields/front-end/gdpr/gdpr-field.php';Add this constant to your configuration file:
define('FORMIDABLE_VIEWS_PATH', '/classes/views');
57-63: Improve return type hint.The return type hint could be more specific to better document the expected structure.
- * @return bool[] + * @return array<string,bool>
71-77: Add file existence check before including view file.The view file inclusion should be guarded to prevent errors if the file is missing.
- include FrmAppHelper::plugin_path() . '/classes/views/frm-fields/back-end/gdpr/primary-options.php'; + $view_file = FrmAppHelper::plugin_path() . '/classes/views/frm-fields/back-end/gdpr/primary-options.php'; + if ( file_exists( $view_file ) ) { + include $view_file; + } else { + error_log( 'GDPR field view file not found: ' . $view_file ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
images/icons.svgis excluded by!**/*.svg,!**/*.svg
📒 Files selected for processing (8)
classes/controllers/FrmHooksController.php(1 hunks)classes/helpers/FrmAppHelper.php(1 hunks)classes/helpers/FrmFieldGdprHelper.php(1 hunks)classes/models/FrmSettings.php(2 hunks)classes/models/fields/FrmFieldGdpr.php(1 hunks)classes/views/frm-fields/back-end/gdpr/primary-options.php(1 hunks)classes/views/frm-fields/front-end/gdpr/gdpr-field.php(1 hunks)classes/views/frm-settings/general.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/models/FrmSettings.php
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (9)
classes/controllers/FrmHooksController.php (1)
115-115: LGTM! Hook registration follows existing patterns.The addition of
FrmFieldGdprHelper::load_hooks()is consistent with the existing architecture and properly placed after other feature-specific hooks.classes/views/frm-fields/front-end/gdpr/gdpr-field.php (2)
8-10: LGTM! Proper security check implemented.The direct access prevention using ABSPATH is correctly implemented.
19-27: LGTM! Good accessibility implementation.The implementation follows accessibility best practices:
- Uses
role="group"for the checkbox container- Implements
aria-labelledbyfor screen readers- Associates label with input using
forattributeclasses/views/frm-fields/back-end/gdpr/primary-options.php (2)
2-11: LGTM! Well-documented code with proper type hints.The file header includes comprehensive documentation with proper
@varannotations for type hinting.
17-18: LGTM! Clear implementation note.Good practice to document the dependency on
FrmFieldGdpr::extra_field_opts().classes/views/frm-settings/general.php (1)
59-64: LGTM! Good implementation of conditional visibility.The GDPR options wrapper properly implements conditional visibility based on the enable_gdpr setting.
classes/helpers/FrmFieldGdprHelper.php (2)
24-32: LGTM!The constants are well-documented and appropriately defined for their purpose.
55-76: LGTM!The field-related methods are well-implemented with proper documentation, strict type comparison, and internationalization support.
classes/models/fields/FrmFieldGdpr.php (1)
35-49: LGTM!The default field settings are well-structured with appropriate default values and proper merging of base and additional settings.
|
I see a funny issue here. If I check out this branch in Lite, but not in Pro, I see "Other" twice, and the first one is empty. Is it possible to do something about that? 
|
|
Hm, the Pro PR doesn't have a lot. I see weird settings even with Pro up to date. 
|
|
Removed the top Other section that would always be empty with 2f7c17a |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
classes/views/frm-settings/general.php (1)
75-80: Consider adding a tooltip for the cookies setting.While the description is clear, consider adding a tooltip (like the IP setting below) to provide more details about:
- What specific cookies are disabled
- Impact on form functionality
- Any recommended alternative approaches
<p <?php FrmAppHelper::array_to_html_params( $gdpr_options_wrapper_params, true ); ?>> <label> <input type="checkbox" name="frm_no_gdpr_cookies" id="frm_no_gdpr_cookies" value="1" <?php checked( $frm_settings->no_gdpr_cookies, 1 ); ?> /> <?php esc_html_e( 'Disable user tracking cookies. This will disable the user tracking feature as well as the ability to limit form entries to one per user by cookie.', 'formidable' ); ?> + <?php FrmAppHelper::tooltip_icon( __( 'This setting affects cookies used for user tracking and entry limiting. Consider implementing alternative methods like user authentication for entry limiting if needed.', 'formidable' ) ); ?> </label> </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/views/frm-settings/general.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
🔇 Additional comments (5)
classes/views/frm-settings/general.php (5)
57-57: LGTM! Well-structured section header.The GDPR section header is properly escaped and uses the correct text domain.
59-67: LGTM! Clean implementation of wrapper logic.The wrapper parameters are well-structured with proper conditional logic for visibility control.
82-87: LGTM! Well-implemented IP storage toggle.The implementation correctly handles:
- GDPR wrapper visibility
- Custom header IP option visibility
- Proper escaping of attributes and text
68-73:Details
❓ Verification inconclusive
Verify the GDPR toggle behavior.
The implementation looks correct, but based on PR comments about duplicate "Other" sections appearing in Lite version, please verify:
- The toggle behavior when Pro is not installed
- The interaction between this toggle and the "Other" section visibility
🏁 Script executed:
#!/bin/bash # Search for other uses of frm_enable_gdpr to ensure consistent behavior rg "frm_enable_gdpr" # Search for other sections with "Other" heading to check for duplicates rg -B 2 -A 2 '"Other"'Length of output: 7186
Verify GDPR Toggle Behavior in Lite Mode
The GDPR toggle in classes/views/frm-settings/general.php appears correctly implemented with proper escaping and the expected use of the data attribute. However, based on previous PR feedback regarding duplicate "Other" sections, please manually verify the following:
- Lite vs. Pro: Confirm that when the Pro version is not installed, toggling the GDPR option properly shows or hides the associated GDPR options.
- "Other" Section Interaction: Ensure there’s no unintended overlap between the GDPR toggle behavior and the visibility or duplication of "Other" field sections. Our search indicates that the GDPR toggle is only used in its designated location, while "Other" options are handled elsewhere, but a thorough manual check is still recommended.
89-90:Details
❓ Verification inconclusive
Investigate "Other" section duplication in Lite version.
Based on PR comments about duplicate "Other" sections in Lite version, verify:
- The conditions controlling this section's visibility
- The interaction with Pro version's "Other" section
🏁 Script executed:
#!/bin/bash # Search for hooks that might be adding duplicate "Other" sections rg -B 5 -A 5 "frm_settings_form|frm_other_settings_form" # Check if there are any Pro-specific conditions around the "Other" section rg "FrmAppHelper::pro_is_installed.*Other"Length of output: 5882
Verify 'Other' Section Visibility Logic in Lite vs. Pro
The "Other" header in
classes/views/frm-settings/general.php(lines 89–90) is rendered unconditionally:<h3><?php esc_html_e( 'Other', 'formidable' ); ?></h3>Immediately after, the code triggers the hook via:
do_action( 'frm_other_settings_form', $frm_settings );Our searches did not reveal any conditional checks (e.g., using
FrmAppHelper::pro_is_installed()) around this header. This absence suggests that in the Lite version the section—and potentially its contents via the hook—will always be rendered, which could lead to the duplicate "Other" sections noted in the PR comments.Action Items:
- Confirm if the header and its associated content should only be rendered when Pro features are enabled.
- Review whether the hooks (
frm_settings_formandfrm_other_settings_form) are intended to segregate Lite and Pro functionalities.- Consider adding a conditional check (possibly leveraging
FrmAppHelper::pro_is_installed()) to prevent the duplication in the Lite version.
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
@Liviu-p I'm seeing some issues with the GDPR field.
- We don't want the format setting.
- Can we come up with some decent default text? I feel like Agreement Text isn't very helpful.
-
It looks like I can uncheck the "Required" setting. This shouldn't be possible. Maybe we can gray it out or hide it?
-
Let's get rid of Default value. This should always be off by default.
-
Placeholder text isn't applicable. Let's remove that setting as well.
-
I also see
Max Charactersis a setting. That shouldn't be there. It seems like we're getting settings for text fields? -
There's also an Invalid Format validation message setting, but that might go away when we get rid of the format setting?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
classes/helpers/FrmAppHelper.php (1)
4414-4424: Correctly implements GDPR cookie restriction check.The
no_gdpr_cookies()method properly checks both required conditions (enable_gdprANDno_gdpr_cookies) before returning true. This implementation addresses the previous concern raised in the review comment about ensuring both conditions are verified.However, the
@since x.xtag should be updated with the actual version number before release.Consider updating the placeholder version number in the method documentation:
- * @since x.x + * @since 6.19(Use the actual version number for this release)
classes/views/frm-fields/back-end/settings.php (1)
88-88: Consider providing a tooltip for the disabled "Required" checkbox.Disabling the checkbox via
<?php disabled( $display['readonly_required'], 1 ); ?>is good practice, preventing contradictory configurations. However, offering a brief explanation (e.g., via a tooltip) would clarify why it cannot be changed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
classes/factories/FrmFieldFactory.php(1 hunks)classes/helpers/FrmAppHelper.php(1 hunks)classes/helpers/FrmFieldGdprHelper.php(1 hunks)classes/models/FrmField.php(1 hunks)classes/models/fields/FrmFieldGdpr.php(1 hunks)classes/models/fields/FrmFieldType.php(1 hunks)classes/views/frm-fields/back-end/settings.php(2 hunks)classes/views/frm-fields/front-end/gdpr/gdpr-field.php(1 hunks)classes/views/frm-settings/general.php(2 hunks)css/frm_admin.css(1 hunks)js/formidable_admin.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- classes/views/frm-fields/front-end/gdpr/gdpr-field.php
- classes/views/frm-settings/general.php
- classes/helpers/FrmFieldGdprHelper.php
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (12)
css/frm_admin.css (1)
7589-7592: Good addition for visually indicating disabled state in dropdowns.This CSS rule properly handles the styling for disabled options in multiselect dropdowns by:
- Reducing the opacity to make disabled options appear faded
- Removing pointer events to prevent mouse interaction with disabled elements
This follows UI best practices for indicating unavailable choices to users while maintaining a consistent look and feel.
classes/models/fields/FrmFieldType.php (1)
339-363: Addition of GDPR-related setting looks good.The new
readonly_requiredfield setting with a default value offalsehas been added to support GDPR compliance functionality. This is a well-implemented change that follows the existing code style and structure.classes/models/FrmField.php (1)
77-81: GDPR field type implementation approved.The addition of the GDPR field type is properly implemented, following the same pattern as other field types in the selection array. It correctly uses the
FrmFieldGdprHelperclass to define the field type constant and control visibility.js/formidable_admin.js (3)
412-413: Appropriate variable declarations for the new unchecking feature.These new variables efficiently handle the attribute parsing. The conditional check ensures
uncheckListArrayis always an array even ifuncheckListis empty or null.
446-453: Well-implemented checkbox uncheck functionality.Good implementation of the GDPR-related functionality to uncheck specified checkboxes. The code:
- Checks if there are checkboxes to uncheck
- Loops through each selector
- Finds and unchecks matching elements
This approach supports the GDPR compliance feature where selecting one option might need to automatically deselect conflicting options.
A potential minor optimization could be to use
document.querySelectorAllwith a comma-separated selector string instead of individualquerySelectorcalls if there are many checkboxes to uncheck, but the current implementation should be efficient for most use cases.
408-456: The new checkbox unchecking feature integrates well with existing code.The GDPR update is properly inserted within the logical flow of the
hideShowItemfunction. The function already handles toggling visibility based on field values, and this enhancement adds the ability to uncheck related checkboxes, which is a natural extension of its functionality.The code maintains the structure and style of the existing implementation, following the established patterns in the codebase.
classes/factories/FrmFieldFactory.php (2)
94-105: No functional changes introduced.These lines appear to be primarily reformatting for alignment and consistency. The cleanup improves readability without altering the underlying behavior.
112-112: Verify GDPR field class resolution.The new mapping for
FrmFieldGdprHelper::FIELD_TYPElooks correct in referencingFrmFieldGdprHelper::get_gdpr_field_class( $field_type ). Make sure the returned class name is valid and properly autoloaded before usage throughout the codebase.classes/views/frm-fields/back-end/settings.php (1)
56-72: Helpful conditional warning for disabled GDPR fields.The newly added block cleanly alerts users when the GDPR field is disabled, directing them to the appropriate settings. This is a user-friendly and well-structured approach.
classes/models/fields/FrmFieldGdpr.php (3)
29-55: Hiding field defaults when GDPR is disabled.The conditional logic that sets all defaults to
falseunderhide_gdpr_field()is consistent with disabling the field. This approach cleanly bypasses extraneous properties when the GDPR field is unsupported.
63-88: Confirm no validation required for hidden fields.When the GDPR field is hidden, all related validations are turned off. This ensures no unexpected errors appear for an effectively disabled field. If future requirements arise, reconsider partial validation rather than outright disabling it.
129-141: Refactor repeated path construction.This code duplicates
FrmAppHelper::plugin_path() . self::VIEW_PATHin bothinclude_form_builder_file()andinclude_front_form_file(). A single helper method could reduce duplication and add path validation.
…r note under general settings, switch block the required checkbox by beign selectable via javascript instead of disabled html attribute as this will make the required to be ignored by the form
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
classes/views/frm-settings/general.php (3)
57-57: Use clear heading titles
It’s great to see a dedicated GDPR section. Consider adding “Settings” to the heading (e.g., “GDPR Settings”) for clarity.
75-80: Refine checkbox label
The label is functional, but consider clarifying implications of disabling tracking cookies for user experience and analytics.
95-100: Add rel attribute for external link
Userel="noopener noreferrer"for security best practices when usingtarget="_blank".- printf( 'Learn more about our GDPR settings <a href="%s" target="_blank">%s</a>', ... + printf( 'Learn more about our GDPR settings <a href="%s" target="_blank" rel="noopener noreferrer">%s</a>', ...classes/models/fields/FrmFieldGdpr.php (4)
1-28: Use a proper @SInCE version
The class definition is good, but replace placeholder version comments (@since x.x) with a real version. This improves maintainability and clarity.
35-55: Localize the default field name
The default name includes “Consent.” To ensure translations, use a localized string instead of a hard-coded English term.- 'name' => $this->get_new_field_name() . ' Consent', + 'name' => $this->get_new_field_name() . ' ' . __( 'Consent', 'formidable' ),
129-131: File inclusion clarity
Returning a direct path to the form builder file is simple and effective. Consider afile_exists()check for safety if the file can ever be missing.
152-157: Optional fallback
Currently hides the GDPR field entirely if it’s disabled, which is logical. Consider a small placeholder or message if you want to explain why the field is hidden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/models/fields/FrmFieldGdpr.php(1 hunks)classes/views/frm-fields/back-end/settings.php(2 hunks)classes/views/frm-settings/general.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/views/frm-fields/back-end/settings.php
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
🔇 Additional comments (9)
classes/views/frm-settings/general.php (4)
58-67: No major concerns
The conditional class toggling for$gdpr_options_wrapper_paramsand$custom_header_ip_wrapper_paramslooks correct and aligns well with the intended UI display logic.
70-70: Nice use of data attributes
Thedata-frmshowanddata-frmuncheckattributes enable clean UI toggling for GDPR settings. No issues spotted.
82-87: Clear IP exclusion option
This checkbox logically hides IP-related options when disabled. The implementation is straightforward and correct.
101-101: Verify “Other” heading duplication
Ensure that switching Lite vs. Pro does not introduce a second, empty “Other” heading. Verify this fits the latest feedback on avoiding duplicate headings.classes/models/fields/FrmFieldGdpr.php (5)
63-88: Looks consistent
Returning different settings based onhide_gdpr_field()aligns with the requirement to hide or show GDPR fields. No issues noted.
90-102: Validate included file path
The code includes a separate file for GDPR primary options. Ensure that file path resolution is always valid and accessible to avoid potential load-time errors.
104-109: Conditional label display
Conditionally showing the label in the form builder is a good approach. No concerns.
117-121: Comprehensive defaults
Providing a default GDPR agreement text is helpful. The text is correctly localized, improving global applicability.
139-141: Consistent front-end inclusion
Same approach for the front-end file. No immediate issues.
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
@Liviu-p Is it possible to make the required checkbox appear greyed out? It looks like I can't disable it, but it doesn't look like a disabled field.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
classes/views/frm-settings/general.php (2)
98-101: Improve translation structure for better internationalizationThe current string construction might cause issues for languages with different sentence structures. Consider using placeholders for better translation flexibility.
- // translators: %s: Knowledge base URL - printf( esc_html__( 'Learn more about our GDPR settings', 'formidable' ) . ' <a href="%s" target="_blank">%s</a>', 'https://formidableforms.com/knowledgebase/gdpr-settings/', esc_html__( 'here', 'formidable' ) ); + // translators: %1$s: Knowledge base URL, %2$s: translated "here" text + printf( esc_html__( 'Learn more about our GDPR settings %2$s', 'formidable' ), 'https://formidableforms.com/knowledgebase/gdpr-settings/', '<a href="' . esc_url( 'https://formidableforms.com/knowledgebase/gdpr-settings/' ) . '" target="_blank">' . esc_html__( 'here', 'formidable' ) . '</a>' );
72-72: Data attribute behavior should be documented for maintainabilityThe
data-frmuncheckattribute contains multiple IDs to uncheck when the GDPR checkbox is checked, creating a complex interaction between settings.Consider adding a code comment explaining this behavior for future maintenance, for example:
+ <!-- When GDPR is enabled, show GDPR options but ensure checkboxes are unchecked initially --> <input type="checkbox" name="frm_enable_gdpr" id="frm_enable_gdpr" value="1" <?php checked( $is_gdpr_enabled, 1 ); ?> data-frmshow=".frm_gdpr_options" data-frmuncheck="#frm_no_gdpr_cookies, #frm_no_ips, #frm_custom_header_ip" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/helpers/FrmAppHelper.php(1 hunks)classes/views/frm-settings/general.php(2 hunks)css/frm_admin.css(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- classes/helpers/FrmAppHelper.php
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
🔇 Additional comments (4)
css/frm_admin.css (1)
6050-6053: LGTM: Appropriate styling for readonly checkboxesThis CSS rule correctly styles readonly checkboxes by reducing opacity and preventing mouse interactions, making it visually clear to users that these elements cannot be interacted with.
classes/views/frm-settings/general.php (3)
5-5: Code looks good!Using
FrmAppHelper::is_gdpr_enabled()to check GDPR status at the beginning ensures consistent behavior throughout the file.
65-68: Good implementation of conditional visibilityThe code correctly handles the nested conditional visibility for the custom header IP container, addressing the previous review comment.
80-80: Clear and specific descriptionThe text has been properly updated to clarify exactly what functionality is disabled when cookies are turned off, addressing the previous review comment.
2 participants
FF PRO - https://github.com/Strategy11/formidable-pro/pull/5585#issuecomment-2678379736