You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add show=label options for fields with options as well.
Test procedure
Create a form and add a Text field, Name field and an option field like Dropdown field.
For the Dropdown field, change one of the labels to a number, like 10 for example.
For the option fields select 'Use separate values' option.
Go to the Text field options and under Advanced > Default Value click on the Calculation tab.
Click on the three dots to trigger the field shortcodes popup and Select 'Text' tab for the calculation.
Confirm that the name field now has three more shortcode options that allows inserting a name part. Ex. show=first
Insert one of the new show shortcodes in the calculation.
Save and Preview the form.
Confirm that everything works as expected.
Turn back to the form builder and check that an additional show=label option is available for the Dropdown field.
Now, change the calculation of the text field to from 'Text' to 'Math' using the tab and insert the show=label shortcode into the calculation and build a number calculation from that adding more operands.
Save and Preview the form.
Confirm that the number calculation works as expected when a number label is selected for the Dropdown field.
Summary by CodeRabbit
New Features
Added shortcode options for individual parts of multi-part fields, including configured first, middle, and last name components.
Added support for extensions to provide shortcode parts for other field types.
Added field type, label, and styling details to generated shortcode options.
Bug Fixes
Preserved custom option labels without truncation.
Corrected the display order and localization of name-part labels.
@AbdiTolesa I was working on adding support for show=first/show=last to the shortcode pop up for name fields to make it easier to insert these special options.
Do you think you'd be able to try taking this over and wrapping it up? I was hoping to also include show="label" for the fields with options that have "Use separate values" enabled as well.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
▶️ Resume reviews
🔍 Trigger review
📝 Walkthrough
Walkthrough
Calculation shortcode generation now uses shared helpers. Name-field parts follow the configured layout. PHP and JavaScript expose extension points for additional field parts. Field labels preserve explicitly provided key labels.
Changes
Multipart calculation shortcodes
Layer / File(s)
Summary
PHP field-part discovery and wiring classes/controllers/FrmFormsController.php, classes/controllers/FrmHooksController.php, classes/helpers/FrmFormsHelper.php
The admin controller resolves Name-field parts from name_layout, accepts filtered parts, and inserts shortcode options. The admin hook registers the field-code-tab callback. Explicit key_label values are no longer truncated.
Calculation popup list generation js/formidable_admin.js, js/src/admin/admin.js
The calculation popup uses shared list-item helpers. Name-field parts are added in layout order, and frm_add_calc_field_shortcodes receives the field, popup, list, and builder callback. Summary lists omit field parts.
Files that changed from the base of the PR and between aa982e7 and d0e5cc0.
Files selected for processing (1)
js/formidable_admin.js (1 hunks)
🔇 Additional comments not posted (2)
js/formidable_admin.js (2)
2905-2907: Ensure browser compatibility
The shouldShowFieldLabelShortcode function uses optional chaining (?.) and includes, which are relatively modern JavaScript features.
Verify that the browsers supported by the Formidable Forms plugin are compatible with these features, or consider providing polyfills or fallback mechanisms for older browsers if necessary.
#!/bin/bash# Check browser compatibility for optional chaining and includes
ast-grep --lang js --pattern $' $$$ (_) ?. (_) $$$'
ast-grep --lang js --pattern $' $$$ (_).includes(_) $$$'
2895-2907: Verify compatibility with existing functionality
The new shouldShowFieldLabelShortcode function introduces new logic for determining whether to show the field label shortcode based on the field type and "Use separate values" checkbox state.
Ensure this change is thoroughly tested and does not introduce any regressions or compatibility issues with existing functionality related to calculation fields.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
js/formidable_admin.js (1)
Line range hint 1-3070: Good use of module pattern, but consider breaking it down
The frmAdminBuildJS function implements the module pattern, which is a good practice for encapsulation and organizing code. It creates a closure that contains private functions and variables, while returning an object with public methods and properties. This approach helps in maintaining a clean global namespace and controlling access to internal functionality.
However, the function is quite large and contains many nested functions and variables. Consider breaking this down into smaller, more focused modules or sub-modules. This could improve readability, maintainability, and make it easier to test individual components.
Here's a high-level suggestion for refactoring:
Identify logical groupings of functionality (e.g., form building, settings, export/import).
Create separate modules for each group.
Use ES6 modules or a similar system to manage dependencies between these modules.
Maintain a main module that initializes and coordinates these sub-modules.
This approach would make the code more modular and easier to maintain in the long run.
Files that changed from the base of the PR and between 8d1c7f5 and f80ffde.
📒 Files selected for processing (1)
js/formidable_admin.js (1 hunks)
🧰 Additional context used🪛 Biome
js/formidable_admin.js
[error] 2867-2868: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
js/formidable_admin.js (2)
2893-2912: Looks good! The new function adds useful functionality.
The maybeAddNamePartShortcodes function is well-structured and adds the ability to insert [nameFieldId show=first], [nameFieldId show=middle] and [nameFieldId show=last] shortcodes to the calculation popup for name fields.
It uses Object.entries and forEach to efficiently loop through the name parts and their labels, and calls addCalcFieldLiToList to add the shortcode <li> elements.
The function is also properly scoped to only run for 'name' field types.
2937-2959: Looks good! The new function improves code modularity.
The new addCalcFieldLiToList function encapsulates the logic for constructing and appending the field shortcode <li> elements to the calculation popup.
It takes the necessary parameters and uses them to build the <a> and <li> elements with the appropriate classes and attributes.
Extracting this logic into a separate function improves code modularity and reusability. The function is also well-documented with JSDoc comments explaining the parameters and return value.
⏰ Context from checks skipped due to timeout of 90000ms (1)
GitHub Check: Cypress
🔇 Additional comments (3)
js/formidable_admin.js (3)
2877-2884: LGTM: Proper field list handling
The field list generation and shortcode support check is implemented correctly. The code properly checks for the data-supportsShowShortcodes attribute before adding additional shortcode options.
2898-2917: LGTM: Well-structured name field shortcode handling
The maybeAddNamePartShortcodes function is well-implemented with:
Clear type checking
Proper field ID validation
Localized labels using __()
Clean iteration over name parts
2942-2965: LGTM: Well-structured list item creation
The addCalcFieldLiToList function follows best practices:
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
js/src/admin/admin.js (4)
3050-3065: Feature flag truthiness can misfire; make it explicitly boolean
dataset.supportsShowShortcodes is a string and any non-empty value (including "0") is truthy. Prefer a presence or explicit boolean parse to avoid accidental enabling.
3113-3145: Minor UI polish: add spacing and accessible label to shortcode items
Currently the label text is concatenated directly after “[code]” with no space, and the link lacks an accessible label. Add a space node and aria-label to improve readability/AT support.
3064-3095: Name parts: consider hiding “Middle” when not enabled on the field
You always add first/middle/last. If the Name field is configured without a middle part, showing “(Middle)” may confuse. Optional: gate on the field’s current layout/settings (if available) before adding that shortcode. I can draft a DOM check if you confirm where the layout is stored.
⏰ 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). (4)
- Added maybeAddAddressPartShortcodes function to support Address field part shortcodes (line1, line2, city, state, zip, country)
- Added function call in field population loop to display Address field parts in calculation popup
- Similar pattern to Name field implementation
Resolves #5825
- Use template literals instead of string concatenation
- Extract addressParts as const variable for better readability
- Add descriptive JSDoc parameter documentation
- Refine function description to be more professional and direct
- Cleaner forEach syntax with consistent formatting
…hortcodes
- Remove hyphens from JSDoc parameter descriptions for consistency with project style
- Align parameter documentation format with standard JSDoc conventions
- Added wp.hooks.doAction('frm_add_calc_field_shortcodes') hook
- Removed maybeAddAddressPartShortcodes() function (Address fields are Pro-only)
- Allows Pro and other extensions to add custom field part shortcodes
- Well-documented hook with JSDoc for extension developers
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
js/src/admin/admin.js (2)
3056-3074: Localize the “(Label)” suffix in label shortcode entry and avoid hard‑coded English
The UI label for the [fieldId show=label] entry is built as fields[ i ].fieldName + ' (Label)', which is not translatable. This should use the existing i18n helpers so the suffix is localized and consistent with the rest of the admin UI.
A minimal change inline with existing patterns:
- if ( shouldShowFieldLabelShortcode( fields[ i ].fieldType, fields[ i ].fieldId ) ) {- addCalcFieldLiToList( list, fieldId, fields[ i ].fieldId + ' show=label', fields[ i ].fieldName + ' (Label)', fields[ i ].fieldType );- }+ if ( shouldShowFieldLabelShortcode( fields[ i ].fieldType, fields[ i ].fieldId ) ) {+ addCalcFieldLiToList(+ list,+ fieldId,+ fields[ i ].fieldId + ' show=label',+ sprintf( __( '%s (Label)', 'formidable' ), fields[ i ].fieldName ),+ fields[ i ].fieldType+ );+ }
This keeps the UX the same while making the suffix properly translatable.
3119-3121: Dropdowns won’t ever show [fieldId show=label] — type check is missing 'select'
shouldShowFieldLabelShortcode only allows ['radio', 'checkbox', 'dropdown']. In this codebase Dropdown fields actually use the type string 'select' elsewhere, so real Dropdown fields will never satisfy this check and their show=label entries won’t appear in the popup.
Recommend broadening the type list so Dropdowns are correctly supported:
-function shouldShowFieldLabelShortcode( fieldType, fieldId ) {- return [ 'radio', 'checkbox', 'dropdown' ].includes( fieldType ) && !! document.getElementById( `separate_value_${ fieldId }` )?.checked;-}+function shouldShowFieldLabelShortcode( fieldType, fieldId ) {+ // Support actual Dropdown type ('select'); keep 'dropdown' in case it exists elsewhere.+ return [ 'radio', 'checkbox', 'select', 'dropdown' ].includes( fieldType ) &&+ !! document.getElementById( `separate_value_${ fieldId }` )?.checked;+}
This aligns the new feature with existing field-type conventions and ensures show=label is available for Dropdowns when “Use separate values” is enabled.
🧹 Nitpick comments (3)
js/src/admin/admin.js (3)
3077-3107: Name-part shortcodes helper looks good; consider aligning with actual name layout (optional)
maybeAddNamePartShortcodes cleanly adds [nameFieldId show=first|middle|last] entries and correctly uses __() for the sublabels, which matches the PR intent.
If you want to tighten UX later, you could read the Name field’s configured layout (eg, which parts are enabled) and skip generating entries for disabled parts so you don’t offer shortcodes like show=middle when no middle-name subfield will ever render. That would avoid confusing, non-functional options in the popup.
3132-3155: Helper for calc-field list items is solid; consider minor accessibility polish
addCalcFieldLiToList centralizes the markup for calc-field entries and correctly stores the raw shortcode in a data-code attribute, which makes the new hook API easier to use and keeps the DOM structure consistent.
Two small optional tweaks you might consider:
Add an href="#" (or role="button" and tabindex="0") to the <a> so it’s keyboard-focusable and announced as interactive even if JS fails.
If you expect add-ons to call this with non-string fieldId, you might document/normalize it to a string to avoid surprising class names.
No blockers here; helper is otherwise well-factored.
3062-3074: Replace @since x.x placeholders before release
The new frm_add_calc_field_shortcodes hook and helper docblocks still use @since x.x. Before tagging a release, these should be updated to the actual version number so downstream developers have accurate version metadata in the public JS API.
Purely documentation/i18n hygiene; no functional impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between cb38709 and 6b72e5c.
We reviewed changes in 70eddfc...56d366e on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
We reviewed changes in e3ee649...be6808a on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
Related update https://github.com/Strategy11/formidable-pro/pull/4573 (merged)
Newer related update https://github.com/Strategy11/formidable-pro/pull/6549
TODO
Test procedure
show=firstshow=labeloption is available for the Dropdown field.Summary by CodeRabbit
New Features
Bug Fixes