[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR changes how the CustomSettings component for Actions v2 is obtained via withFilters in modules/actions-v2/assets/src/helpers/withActionLocalizeScript.js. The withFilters(...)() call was moved out of the inner render function (so CustomSettings is created once per HOC) to avoid remounting on each render — this is intended to preserve input focus in custom action settings. The build asset version was bumped in modules/actions-v2/assets/build/editor.asset.php.

What I like

• Creating the CustomSettings component once (stable identity) is the right idea to avoid input remounts and lost focus.
• The asset version bump is expected when JS changes.

Issues / Risks

1. Potential filter registration timing issue

   • Previously withFilters(...) was invoked inside the returned render function, i.e. at render-time. Now it is invoked when withActionLocalizeScript runs (the HOC creation time). If other code registers filters for "jet.fb.render.action.{actionType}" after withActionLocalizeScript runs, those filters will not be applied to the CustomSettings component because the filtered component has already been created. That can cause custom settings to not be injected.
   • Recommend reviewing when withActionLocalizeScript is called in the plugin life-cycle relative to other modules that may register the filter. If the HOC is created early (during module import / registration), this change could break third-party or late-loading integrations that hook the filter later.

2. Alternative safer implementation suggestion

   • To preserve focus while ensuring filters are applied after all registrations, consider creating the filtered component on the first render and then keeping its identity stable, for example using useRef or useMemo inside the returned component:
     • useRef: if (!ref.current) { ref.current = withFilters(...)(() => null); } then use ref.current in JSX. This ensures the filtered component is resolved once per action instance but the resolution happens at render-time (after other scripts have had a chance to register filters), and the identity is stable across re-renders.
     • useMemo with appropriate dependency (empty for single-instance memoization) can also work but be mindful of server-side rendering and hook rules.

3. Minor coding style

   • The new eslint-disable comment (@wordpress/no-unused-vars-before-return) is acceptable but points to a lint/ordering concern. If adopting the useRef pattern above, that eslint suppression would not be necessary.
   • Ensure files end with a newline to follow common style checks. The patch shows "No newline at end of file" for the JS file earlier; please make sure the final file ends with a newline.

Security / Performance / Back-compat

• Security: no issues introduced (no direct user input handling). Nonce/capability rules unaffected.
• Performance: reducing re-creation of the CustomSettings component improves render performance and avoids unnecessary remounts.
• Backward compatibility: potential breaking behavior for third-party code that registers the filter later than the HOC creation. Treat as a compatibility risk — see suggestions above to mitigate.

Tests

• There are no automated tests added. This is a behavioral change affecting integrations; consider adding a small JS test (or manual QA steps) that verifies: (a) a third-party filter applied before and after HOC creation still injects CustomSettings, and (b) focus is preserved when interacting with inputs in custom settings.

Requested changes

• Either:
  a) Move the withFilters(...) invocation into the first-render (useRef) so filters are applied at render-time but the component identity is stable; or
  b) If keeping the current approach, add a comment explaining why this is safe (i.e. guarantee that withActionLocalizeScript is always called after plugins register their filters), and add a newline at end of file.

Files of interest

• modules/actions-v2/assets/src/helpers/withActionLocalizeScript.js — main change, see comments above.
• modules/actions-v2/assets/build/editor.asset.php — updated build version (expected).

Overall

• The intent is correct and this likely fixes the focus loss issue. However, please address the filter timing concern (useRef-based initialization recommended) to avoid breaking integrations that register filters later.

Suggested changelog entry

- FIX: preserve input focus in custom action settings (Actions v2 editor)