Skip to content

FIX: Scope radio auto-transition to enabled fields only https://github.com/Crocoblock/issues-tracker/issues/18464#640

Merged
Gawuww merged 3 commits into
release/3.6.1from
issue/18464
May 26, 2026
Merged

FIX: Scope radio auto-transition to enabled fields only https://github.com/Crocoblock/issues-tracker/issues/18464#640
Gawuww merged 3 commits into
release/3.6.1from
issue/18464

Conversation

@Gawuww
Copy link
Copy Markdown
Collaborator

@Gawuww Gawuww commented May 11, 2026

@github-actions
Copy link
Copy Markdown

🤖 AI PR Review

Risk level: medium

Review

Summary

  • This PR scopes the automatic page transition triggered by radio inputs to only work for enabled "switch-on-change" fields and prevents attempting to advance when already on the last page.

What changed (important files)

  • modules/switch-page-on-change/assets/src/frontend/main.js
    • Adds getSwitchWrapper() and explicit checks so click handling only triggers if the radio is inside a .jet-fb-switch-page-on-change wrapper that belongs to the current page scope. Consolidates changePage() calls into a single helper.
  • modules/switch-page-on-change/assets/build/frontend.js
    • Built counterpart updated accordingly.
  • assets/src/frontend/multi.step/PageState.js
    • Early return added in PageState.changePage() if this.isLast() to avoid attempting to move past the last page.
  • assets/build/frontend/multi.step.js, other build files
    • Built assets updated to reflect source changes.

Positive notes

  • The change fixes the bug reported in the issue where radios outside of enabled switch wrappers caused automatic transitions. Scoping the handler to closest('.jet-fb-switch-page-on-change') and checking page.isNodeBelongThis(...) is the correct approach to limit the effect only to configured fields.
  • Preventing advancing from the last page is a sensible safety guard.
  • The code refactor reduces duplication (single changePage helper) and improves readability.

Security, performance, compatibility concerns

  • Security: front-end JS change only. No server-side security implications.
  • Performance: negligible. Event delegation and closest() usage are fine.
  • Backward compatibility: This is a behavior change and may affect sites that currently rely on the previous (broader) behavior where any radio could trigger a page change. In most cases this is desirable (fix), but call-out to owners:
    • If a site relied on radios outside of .jet-fb-switch-page-on-change to trigger page switching, they will no longer trigger it. Make sure all fields intended to auto-transition have the correct wrapper/class or configuration.

Potential issues and recommendations

  • Tests: There are no unit/browser tests added. Please add (or request QA) tests for the multi-step behaviour and switch-on-change module. Suggested test cases:

    • Radio inside .jet-fb-switch-page-on-change toggles page when checked (both with and without .check-mark-control).
    • Radio outside the switch wrapper does not trigger a page change.
    • Ensure jfbSync watchers still trigger changePage when the field value becomes non-empty.
    • Ensure last-page behaviour: clicking a switch on the last page should not attempt to advance or cause JS errors.
  • Edge cases to verify manually/with tests:

    • Radios nested inside the switch wrapper but where the wrapper is outside the current page scope (the new page.isNodeBelongThis check should prevent cross-page triggers).
    • Custom fields or 3rd-party integrations that may set jfbSync on non-input nodes; ensure they still behave as expected.
  • Built assets: The PR updates built assets. Ensure the build pipeline (npm/yarn) and versioning are correct and that source-to-build mapping is accurate before merge.

Minor notes

  • Files show No newline at end of file in some built assets — not critical but consider normalizing.

Overall

  • This is a targeted, low-impact fix that addresses the reported issue. Approve after adding/confirming tests and ensuring consumers are aware of the behavioral tightening (scoping to .jet-fb-switch-page-on-change).

Suggested changelog entry

- FIX: Scope radio auto-transition to enabled fields only for multi-step forms (switch-page-on-change)

@github-actions
Copy link
Copy Markdown

🤖 AI PR Review

Risk level: low

Review

Summary

  • This PR scopes the “radio -> auto-advance” behavior to only the fields explicitly enabled for switch-on-change and prevents moving forward when already on the last page.

What changed (important files)

  • assets/src/frontend/multi.step/PageState.js
    • Adds an early return if this.isLast() in PageState.changePage(), preventing advancing beyond the last page.
  • modules/switch-page-on-change/assets/src/frontend/main.js
    • Click handler now only reacts to radios contained in a .jet-fb-switch-page-on-change wrapper and confirms the wrapper belongs to the current page (page.isNodeBelongThis).
    • Introduces helper for changePage() and getSwitchWrapper() for clarity and re-use.
    • Keeps the jfbSync watcher logic but limits it to wrappers that belong to the page.
  • Built assets updated to reflect the above JS changes (assets/build/... and modules/.../assets/build/...).

Security

  • No server-side changes, so no nonce/capability impacts introduced by this PR.
  • The additional check page.isNodeBelongThis(...) is good: it prevents cross-page clicks from triggering transitions on other pages (reduces chance of cross-widget interference). No security concerns seen.

Behavior & backward compatibility

  • This is a behaviour change: previously any radio click on the page could trigger a page switch; now only radios inside elements with class .jet-fb-switch-page-on-change (the module’s intended wrapper) will trigger auto-transition. This is likely the desired bugfix but may break sites that relied on the previous global behavior.
    • Action: document the change in release notes and mention that themes/templates must include .jet-fb-switch-page-on-change on fields that should auto-advance.
  • The isLast() guard prevents advancing past the last page. That’s correct and avoids index overflow/inconsistent states. Good safety fix.

Performance

  • No new heavy operations. Event delegation continues to be used correctly. Limiting listeners by checking the wrapper/class early may be slightly more performant.

Edge cases and tests I recommend

  • Add automated / integration tests (or at least QA check list) covering:
    • Radio inside .jet-fb-switch-page-on-change should trigger change when checked.
    • Radio outside that wrapper should NOT trigger change.
    • Radios with check-mark-control inside the wrapper still behave the same (immediate change when checked).
    • Radios on last page do not attempt to advance (confirm no errors / index changes).
    • Multi-form pages on same screen: clicking a radio in one form should not affect the other form.
    • Disabled radios (input.disabled) should not trigger a transition; consider adding an explicit guard (if event.target.disabled return) to the click handler to make intent explicit and avoid relying on browser event behavior.

Code quality / style notes

  • The JS changes are readable and introduce small helpers which improve maintainability.
  • Built files updated accordingly — ensure build pipeline is used so no mismatch between src and build files.
  • Minor: several built files show “No newline at end of file” — not critical but nice to fix in build output.

Recommendations before merge

  1. Add or update a short unit/integration test for the switch-page-on-change module (behavior described above). High-value given this is a UX change.
  2. Add an explicit guard for disabled inputs in the click handler (e.g. if ( event.target.disabled ) return; ) to make the intent explicit and avoid relying on click events on disabled elements.
  3. Update docs or admin UI help text to note that only fields wrapped with .jet-fb-switch-page-on-change will auto-transition now.
  4. Verify build process regenerated all compiled assets (and remove trailing newline issues if possible).

Overall

  • The change is focused, fixes the reported issue, and improves safety around page transitions. The risk is low but it changes visible behavior so add tests and update docs to avoid surprises for integrators.

Suggested changelog entry

- FIX: Scope radio auto-transition to enabled fields only (multi-step forms: switch-page-on-change)

@Gawuww Gawuww merged commit bb880e2 into release/3.6.1 May 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant