[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR implements min/max selection limits for option fields (choices/checkbox/select), adding editor controls, front-end enforcement, accessibility/error UI and integration with select2/autocomplete. Key additions live in modules/option-field (selection-limit JS/CSS and attr handling) and modules/advanced-choices (editor controls + render wrapper attributes). Several built assets and small compatibility patches were also added.

What I checked

• Front-end enforcement logic: modules/option-field/assets/src/frontend/selection-limit (g class) and integration into InputData (choices/checkbox) to show/clear errors and to disable options when max reached.
• Editor UI: modules/advanced-choices/assets/src/editor/choices-field/edit.js imports SelectionLimitsControls and adds it to inspector controls.
• Server-side: advanced-choices/block-renders/choices-field-render.php now merges wrapper attributes from \JFB_Modules\Option_Field\Selection_Limit::get_attributes(...).
• Compatibility with select2/autocomplete: selection-limit checks for select2 and adapts by hooking into select2 events and adjusting maximumSelectionLength.
• Asset changes and webpack tweak (babel config) in modules/advanced-choices/webpack.config.js.

Positive

• Accessibility is considered: error node uses role="alert", manages aria-invalid and aria-describedby.
• Select2 integration handled, and the code attempts to clean up event listeners and MutationObserver in destroy().
• Uses textContent for error messages (safe from XSS on the client side if server provided message is plain text).

Issues / Recommendations

1. Server-side escaping & sanitization (HIGH priority)

   • The PHP change in block-renders (block-renders/choices-field-render.php) calls \JFB_Modules\Option_Field\Selection_Limit::get_attributes(...). Ensure that function: a) only outputs attributes when values present, b) properly sanitizes/escapes values for HTML attributes (esc_attr, esc_html as appropriate) and c) sanitizes messages used for data attributes. If those values are used later as text for aria/error messages, they must be safe. If Selection_Limit::get_attributes isn't already escaping, please add escaping and input validation.
   • Server-side validation: selection limits are a client-side convenience, but critical validation must also be enforced server-side when processing submissions. I didn't see any server-side validation in this diff — confirm that form submission sanitizers/validators include min/max selection checks to prevent users bypassing client-side restrictions.

2. Editor build / dependencies (medium)

   • modules/advanced-choices/assets/src/editor/choices-field/edit.js imports SelectionLimitsControls from the option-field module (relative path). Ensure the editor build process bundles/exports the option-field module into the advanced-choices editor build (or register correct script dependencies) so the control is always available in the editor. If not bundled, the block may fatal in the editor. Verify assets registration (editor.asset.php) includes the necessary handles or that webpack bundling pulls it in.

3. Asset handles / script dependencies (low/medium)

   • Several asset manifest files (.asset.php) changed versions/dependencies. Ensure enqueueing order is correct and option-field scripts/styles are loaded before blocks that rely on them.

4. Performance / scalability (medium)

   • The selection-limit controller uses MutationObserver and iterates over all inputs/options at update(). For very large option lists (hundreds/thousands of choices) this could be heavy. Suggestions:
     • Bail out early where possible (the code already only instantiates selection-limit if min/max exist, good). Consider throttling updates or minimizing DOM queries (cache results where possible).
     • For large select lists with select2, ensure select2's own maximumSelectionLength option is used (the code already sets it via dataAdapter when possible). That's good.

5. Keyboard accessibility and focus edge-cases (low/medium)

   • Disabling individual inputs when max is reached may affect keyboard users: ensure tab-order and keyboard navigation continue to work seamlessly when options become disabled. The code attempts to manage tabindex in signals for choice fields, but please manually test keyboard flows (arrow navigation, space/enter) when options are disabled and when the error is shown.

6. Dynamic data / preset changes (low)

   • Many forms use dynamic presets or update options via JS (AJAX). The MutationObserver watches childList/subtree, so that should generally handle dynamically-added options. Still, please test dynamic scenarios: dynamic preset updates, options coming from JetEngine relations, remote-loaded select2 options.

7. Select2 & jQuery dependency (low)

   • The select2 integration uses jQuery to find select2 instance (jQuery(...).data('select2')). If select2 is not present or if another select replacement is used, behavior should fall back gracefully (code already checks for data). Ensure no console errors in absence of select2.

8. BrowserReporting changes

   • assets/src/frontend/main/reporting/BrowserReporting.js: introduced reportedInline state and calls input.showError/clearError. Good, but be sure Input implementations (all inputs) support showError/clearError or calls are guarded (code uses ?. operator in some places; ensure build target supports optional chaining or is transpiled). Also ensure reportedInline is reset after clearReport — the change sets it false which is fine.

9. Webpack/babel change

   • modules/advanced-choices/webpack.config.js changes babel-loader to specify configFile with path.resolve(...). Ensure path is valid in CI/build environments. This change could break local builds if path assumptions differ. Confirm tests/build on CI.

10. Tests & QA (recommended)

• Please add at least one integration test or QA checklist covering:
  • Checkbox field: min and max limits together with add-new-custom-option, including server validation on submit.
  • Choice/radio groups keyboard navigation when options are disabled.
  • Select (select2) with maximumSelectionLength enforced and selecting/unselecting behavior.
  • Dynamic population of options (AJAX/preset) and updates to limits/messages.

Files to review closely before merge

• modules/option-field/assets/src/frontend/selection-limit/* (core behavior; ensure cleanup/escaping)
• modules/option-field/assets/build/checkbox.js and checkbox.css (ensure styles don't conflict and are scoped)
• modules/advanced-choices/block-renders/choices-field-render.php (server-side attribute output — must be escaped)
• modules/advanced-choices/assets/src/editor/choices-field/edit.js (ensure editor bundling/deps)
• modules/advanced-choices/webpack.config.js (build path change)

Missing tests

• No automated tests added for selection-limit behavior. Add integration tests or at least JS tests for the controller and a PHP test for server-side validation.

Conclusion / Risk summary

• This is a well-structured feature with accessibility and select2 support. The main outstanding concerns are server-side sanitization and enforcing limits on submission (prevent bypass). Also verify editor imports/bundling and webpack config changes.

Requested changes (short)

• Ensure Selection_Limit::get_attributes sanitizes/escapes data attributes and messages.
• Enforce min/max constraints server-side on submission handling.
• Verify editor build includes option-field controls or add explicit script dependency.
• Add/mention tests for key interaction scenarios.

If you want I can propose the exact PHP escaping changes or a unit test skeleton for submission validation.

Suggested changelog entry

- ADD: min/max selection limits for option fields (choices/checkbox/select) — editor controls, front-end enforcement and select2 support