-
Notifications
You must be signed in to change notification settings - Fork 21
UPD: Add form macros logic https://github.com/Crocoblock/issues-track… #607
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
base: release/3.6.0
Are you sure you want to change the base?
Conversation
🤖 AI PR ReviewRisk level: ReviewThis PR adds form macros logic with a sizable update to CSS and JavaScript frontend assets, including calculated fields, conditional blocks, dynamic values, and media/file upload components. Security: The JS code uses sanitized input and existing JetFormBuilder APIs. Calculated field formulas and dynamic values rely on safe parsing (e.g., use of eval is concerning but expected to be controlled in formula contexts); allowlist or validation for formulas and macros should be ensured elsewhere. No direct nonce or capability checks are impacted here as this is frontend logic. Performance: The frontend JS handles reactive inputs, conditions, and dynamic updates efficiently with debouncing and caching strategies, and there's careful handling of file previews and sorting. Backward Compatibility: The PR appears to extend existing APIs and patterns without breaking changes. Core class prototypes are extended properly. It includes new conditional checkers and integration with macros logic. Styling: Large CSS changes are mostly layout and UI improvements; no regressions detected. Adheres to WP coding standards. Testing: No explicit tests are included or referenced in the diff, especially for new or updated JS modules (calculated field formatting, conditional logic, dynamic values). Given the complexity and potential edge cases in formula evaluation and conditional display, thorough automated and manual tests should be ensured. Multisite & Integration: No direct conflicts or issues detected, integration with Gutenberg and JetEngine data expected to continue as normal. Recommendations:
Overall, the changes align well with JetFormBuilder architecture and standards, representing an improvement and enhancement to macros and form field dynamics. Suggested changelog entry
|
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.
Pull request overview
Adds “form macros” support in the editor and runtime, including a new Macros Inserter module and repeater-aware macro value resolution.
Changes:
- Register a new
Macros_Insertermodule and add tooling (webpack/babel) for its editor script. - Enrich macro-field UI with repeater context and add frontend logic to resolve repeater macro output.
- Rebuild multiple compiled frontend/editor assets to include the new behavior.
Reviewed changes
Copilot reviewed 45 out of 57 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/modules-controller.php | Registers the new Macros_Inserter module in the modules controller. |
| modules/macros-inserter/webpack.config.js | Adds webpack config for building the macros inserter editor bundle. |
| modules/macros-inserter/package.json | Adds build/dev scripts for the macros inserter module. |
| modules/macros-inserter/module.php | Enqueues the macros inserter editor script via the module lifecycle hooks. |
| modules/macros-inserter/assets/src/editor.js | Adds BlockEdit toolbar UI to insert macro placeholders into core/html blocks. |
| modules/macros-inserter/.babelrc | Reuses the root babel configuration for module builds. |
| modules/html-parser/assets/build/parser.asset.php | Build metadata version bump from asset rebuild. |
| modules/html-parser/assets/build/admin-ui.asset.php | Build metadata version bump from asset rebuild. |
| modules/blocks-v2/repeater-field/assets/src/frontend/repeater-macros.js | Adds repeater macro value resolver and change notification binding. |
| modules/blocks-v2/repeater-field/assets/src/frontend/index.js | Hooks repeater macro resolver into the macro value filter. |
| modules/blocks-v2/repeater-field/assets/build/frontend.css | Rebuilt CSS output for repeater-field frontend bundle. |
| modules/blocks-v2/repeater-field/assets/build/frontend.asset.php | Build metadata version bump from asset rebuild. |
| modules/blocks-v2/repeater-field/assets/build/editor.asset.php | Build metadata version bump from asset rebuild. |
| modules/blocks-v2/actions-integration/assets/build/editor.js | Rebuilt actions-integration editor bundle output. |
| modules/blocks-v2/actions-integration/assets/build/editor.asset.php | Build metadata version bump from asset rebuild. |
| jet-form-builder.php | Updates plugin version constant. |
| assets/src/package/macros.button/components/MacrosFields.js | Enriches macro fields with repeater context based on block tree. |
| assets/src/package/macros.button/components/MacroFieldItem.js | Updates macro list item labeling/click payload to include context. |
| assets/src/frontend/main/html.macro/observeNode.js | Adds macro host template caching + newline conversion behavior. |
| assets/src/frontend/main/html.macro/CalculatedHtmlString.js | Passes macroHost through macro field value filters for context-aware resolving. |
| assets/build/frontend/multi.step.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/media.field.restrictions.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/media.field.js | Rebuilt media field frontend bundle output. |
| assets/build/frontend/media.field.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/main.css | Rebuilt frontend CSS output. |
| assets/build/frontend/main.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/dynamic.value.js | Rebuilt dynamic value frontend bundle output. |
| assets/build/frontend/dynamic.value.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/conditional.block.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/calculated.field.js | Rebuilt calculated field frontend bundle output. |
| assets/build/frontend/calculated.field.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/frontend/advanced.reporting.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/editor/package.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/editor/form.builder.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/editor/default.builder.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/admin/vuex.package.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/admin/pages/jfb-settings.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/admin/pages/jfb-addons.asset.php | Build metadata version bump from asset rebuild. |
| assets/build/admin/package.asset.php | Build metadata version bump from asset rebuild. |
Comments suppressed due to low confidence (7)
jet-form-builder.php:1
- The plugin version constant was changed to an invalid-looking value (
3.5.6111111111111212). This will leak into asset versioning/cache busting and may break semantic-version expectations elsewhere. Please set it to the intended release version (e.g.,3.5.7or similar) and keep it consistent with your release process.
modules/macros-inserter/webpack.config.js:1 - Production mode detection is incorrect: webpack CLI typically passes
--mode production(separate args), so--mode:productionwill never match anddevModewill stay true even in production. This likely enables inline source maps for production builds. Consider readingprocess.argvfor--modeand its next argument, or rely onprocess.env.NODE_ENV/argv.mode.
modules/macros-inserter/module.php:1 - This module enqueues
assets/build/editor.js, but this PR only addsassets/src/editor.jsand does not add the builtassets/build/editor.jsartifact. If build artifacts are committed in this repo, this will result in a 404 and the feature won’t load. Please add the built file (and corresponding.asset.phpif required by your conventions) or update the enqueue path to match the actual packaged output.
modules/macros-inserter/module.php:1 - The enqueued editor script depends on WP packages (
wp.hooks,wp.compose,wp.element,wp.blockEditor,wp.components). Passing an empty dependencies array can cause load-order issues. Consider generating and using aneditor.asset.php(WP build convention) or explicitly listing dependencies likewp-hooks,wp-compose,wp-element,wp-block-editor,wp-components.
modules/macros-inserter/assets/src/editor.js:1 datais declared but never used. Please remove it to reduce noise and avoid confusing readers about intended behavior.
modules/macros-inserter/assets/src/editor.js:1- This builds HTML via string interpolation and injects values derived from block attributes (
field.repeater_name) directly into HTML. If those values contain<,\", etc., this can produce HTML injection/XSS when inserted into the HTML block content. Prefer building DOM nodes (e.g.,document.createElement) and settingtextContent/setAttribute, or escape/encode interpolated values before concatenation.
modules/blocks-v2/repeater-field/assets/src/frontend/repeater-macros.js:1 - This returns HTML assembled from user-controlled field values and joins with
<br/>. If a field value contains HTML (e.g.,<img onerror=...>), it will be injected into the page when the macro output is applied viainnerHTML, creating an XSS vector. Escapename/valuefor HTML output (convert<,>,&,\",') or return plain text and render line breaks safely.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') { | ||
| const millis = eval(millisInput); | ||
| if (!millis || isNaN(millis) || null === millis || 0 === millis) { | ||
| return 0; | ||
| } | ||
| const date = new Date(millis); |
Copilot
AI
Jan 28, 2026
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.
Using eval() on millisInput is a critical code injection risk. If millisInput can be influenced by a formula or user input, this can execute arbitrary JavaScript in the browser context. Replace eval with safe numeric conversion (e.g., Number(...) / parseInt) or a restricted expression parser if expressions must be supported.
| function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') { | |
| const millis = eval(millisInput); | |
| if (!millis || isNaN(millis) || null === millis || 0 === millis) { | |
| return 0; | |
| } | |
| const date = new Date(millis); | |
| function convertMillisToDateString(millisInput, format = 'YYYY-MM-DD') { | |
| const millis = Number(millisInput); | |
| if (!millis || isNaN(millis) || null === millis || 0 === millis) { | |
| return 0; | |
| } | |
| const date = new Date(millis); |
…er/issues/5654