Prepare for 6.24#2488
Conversation
WalkthroughVersion bump to 6.24 across metadata and docs, plus packaging tweaks. Added export-ignore entries and zip exclusions. Updated changelog/readme. Numerous @since/@deprecated docblocks updated to 6.24. No runtime or control-flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Invalid configuration: Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/helpers/FrmHtmlHelper.php (1)
91-107: Fix undefined $selected_unit notice and make unit parsing safer.$selected_unit is used before it’s guaranteed to be defined (Line 103), which triggers notices when $value is numeric/empty. Also, make preg_quote delimiter-safe.
Apply:
@@ - $units = $args['units']; - $value = $args['value']; + $units = $args['units']; + $value = $args['value']; + $selected_unit = $args['default_unit']; @@ - // Extract unit and number value if value is a string + // Extract unit and number value if value is a string if ( '' !== $value && ! is_numeric( $value ) ) { - $pattern = '/^([0-9.]*)(' . implode( '|', array_map( 'preg_quote', $units ) ) . ')?$/'; + $pattern = '/^([0-9.]*)(' . implode( '|', array_map( static function ( $u ) { return preg_quote( $u, '/' ); }, $units ) ) . ')?$/'; preg_match( $pattern, $value, $matches ); - $selected_unit = $matches[2] ?? ''; + $selected_unit = $matches[2] ?? $selected_unit; if ( ! empty( $matches[1] ) ) { $value = $matches[1]; } } @@ - 'type' => ! empty( $selected_unit ) ? 'number' : 'text', + 'type' => ( ! empty( $selected_unit ) ? 'number' : 'text' ),
🧹 Nitpick comments (6)
js/src/admin/admin.js (1)
10166-10208: Harden entries screen “max columns” helper and avoid initial flash.
- Guard against missing to prevent a potential NPE.
- Start the note hidden to avoid flicker before the first check runs.
- const maxSelectionsNote = div( { - className: 'frm_warning_style', + const maxSelectionsNote = div( { + className: 'frm_warning_style frm_hidden', text: __( 'Only 10 columns can be selected at a time.', 'formidable' ), } ); maxSelectionsNote.style.margin = 0; - const legend = screenOptionsWrapper.querySelector( 'legend' ); - legend.parentNode.insertBefore( maxSelectionsNote, legend.nextElementSibling ); + const legend = screenOptionsWrapper.querySelector( 'legend' ); + if ( legend && legend.parentNode ) { + legend.parentNode.insertBefore( maxSelectionsNote, legend.nextElementSibling ); + } else { + // Fallback: prepend to wrapper if legend is not present. + screenOptionsWrapper.prepend( maxSelectionsNote ); + }.gitattributes (1)
44-45: Consider mirroring export-ignore in the zip script for parity.You added export-ignore for webpack.dev.js and .browserslistrc. The packaging script excludes webpack.dev.js but not .browserslistrc. If you want identical artifacts between “Download ZIP” and release zips, add .browserslistrc to bin/zip-plugin.sh as well.
Apply in bin/zip-plugin.sh near other -x lines:
- -x "*/webpack.dev.js" + -x "*/webpack.dev.js" \ + -x "*/.browserslistrc"bin/zip-plugin.sh (1)
98-101: Packaging exclusions look correct; minor parity tweak suggested.Exclusions for formidable-ai/resources/* and webpack.dev.js are good. Consider also excluding .browserslistrc for parity with .gitattributes and quoting vars to avoid path issues.
-zip -r $zipname $destination \ +zip -r "$zipname" "$destination" \ @@ - -x "*/webpack.dev.js" + -x "*/webpack.dev.js" \ + -x "*/.browserslistrc" @@ -rm -rf $zipname +rm -rf "$zipname" @@ - rm -rf $destination - rsync -avz --exclude 'node_modules' --exclude '*.git/*' --exclude 'tests' $source/ $destination/ + rm -rf "$destination" + rsync -avz --exclude 'node_modules' --exclude '*.git/*' --exclude 'tests' "$source/" "$destination/" @@ -if [ ! -z "$3" ]; then - rm -rf $destination +if [ ! -z "$3" ]; then + rm -rf "$destination"classes/views/styles/components/FrmTextToggleStyleComponent.php (1)
20-21: Consider adding constructor param annotations for clarity.Adding @param types here would help static analysis and IDEs.
/** * Construct FrmTextToggleStyleComponent. * * @since 6.24 + * @param string $field_name + * @param mixed $field_value + * @param array $data */ public function __construct( $field_name, $field_value, $data ) {classes/views/frm-fields/back-end/settings.php (1)
213-215: BC note updated — verify the fallback is still needed.If all supported Pro versions include FrmProHtmlHelper::echo_radio_group, consider deprecating/removing the BC branch in a future release and noting a removal target.
readme.txt (1)
375-384: Changelog block mirrors changelog.txt — LGTM.Optionally add an “Upgrade Notice” for 6.24 highlighting the Field settings redesign.
== Upgrade Notice == - = 6.20 = - This version fixes a security-related bug. Upgrade immediately. + = 6.24 = + UI refresh for Field settings plus multiple fixes. No breaking changes expected. + + = 6.20 = + This version fixes a security-related bug. Upgrade immediately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
js/formidable.min.jsis excluded by!**/*.min.js,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonstripe/js/frmstrp.min.jsis excluded by!**/*.min.js,!**/*.min.js
📒 Files selected for processing (16)
.gitattributes(1 hunks)bin/zip-plugin.sh(1 hunks)changelog.txt(1 hunks)classes/controllers/FrmAppController.php(1 hunks)classes/helpers/FrmAppHelper.php(2 hunks)classes/helpers/FrmFieldsHelper.php(3 hunks)classes/helpers/FrmHtmlHelper.php(1 hunks)classes/helpers/FrmStylesHelper.php(1 hunks)classes/models/fields/FrmFieldType.php(1 hunks)classes/views/frm-fields/back-end/settings.php(1 hunks)classes/views/styles/components/FrmTextToggleStyleComponent.php(2 hunks)formidable.php(1 hunks)js/src/admin/admin.js(2 hunks)readme.txt(2 hunks)resources/scss/admin/backwards-compatibility/add-ons/_surveys.scss(1 hunks)resources/scss/admin/backwards-compatibility/pages/_builder.scss(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cypress
🔇 Additional comments (14)
classes/controllers/FrmAppController.php (2)
794-800: Docblock bump to 6.24 looks goodThe @SInCE tag now reflects the 6.24 release; no runtime change.
801-810: Assets exist and are correctly packaged
css/admin/frm-settings-components.css and js/formidable-settings-components.js are present in the repo and not excluded by .gitattributes or bin/zip-plugin.sh.formidable.php (1)
5-5: Version header bumped to 6.24Header version matches the release target. Ensure FrmAppHelper::plugin_version() returns 6.24 so admin asset cache-busting stays consistent.
resources/scss/admin/backwards-compatibility/add-ons/_surveys.scss (1)
1-1: Comment tag updated to @SInCE 6.24Purely informational; no CSS changes.
js/src/admin/admin.js (1)
1028-1049: Docblock bump to 6.24 looks good.The annotation aligns with the helper’s purpose. No functional changes needed.
classes/helpers/FrmStylesHelper.php (1)
457-457: Docblock version bump looks good.No runtime changes here.
classes/helpers/FrmHtmlHelper.php (1)
62-62: Docblock update is fine.No behavioral change here.
resources/scss/admin/backwards-compatibility/pages/_builder.scss (1)
1-1: Comment-only update acknowledged.No CSS changes; safe to merge.
Also applies to: 26-26
classes/models/fields/FrmFieldType.php (1)
1835-1839: Deprecation metadata aligned with 6.24.Matches the docblock and _deprecated_function call.
classes/helpers/FrmFieldsHelper.php (1)
2434-2435: Docblock version bumps are fine.No logic changes; AI button behavior remains unchanged.
Also applies to: 2458-2459, 2484-2484
classes/helpers/FrmAppHelper.php (2)
32-32: Version bump to 6.24 is correct.Consistent with release prep.
4606-4619: UTF-8 helper looks good.Graceful fallback to seems_utf8 on WP < 6.9; returns false if neither exists, which is acceptable for our supported WP range.
classes/views/styles/components/FrmTextToggleStyleComponent.php (1)
11-14: Docblock @SInCE looks correct for 6.24.No runtime changes; metadata aligns with the release bump.
changelog.txt (1)
2-10: 6.24 entry reads well and matches the release theme.Looks consistent with readme and no formatting issues spotted.
No description provided.