Split out legacy views from admin JS#2331
Conversation
WalkthroughThis change migrates all legacy view-related JavaScript logic out of Changes
Sequence Diagram(s)sequenceDiagram
participant AdminPage as WP Admin (Legacy Views)
participant FrmAppController as FrmAppController (PHP)
participant Browser as Browser (JS)
participant Server as WP AJAX
AdminPage->>FrmAppController: Load legacy views admin page
FrmAppController->>Browser: Enqueue legacy-views.js
Browser->>Browser: Initialize legacy-views.js (UI handlers, validation, AJAX)
Browser->>Server: (On user action) AJAX request (e.g., add row, update form)
Server-->>Browser: AJAX response (HTML, selectors, etc.)
Browser->>Browser: Update UI dynamically
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)js/formidable_admin.js (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
classes/controllers/FrmAppController.php (1)
873-875: Explicitly enqueue the script in the footer to keep dependency order predictable
formidable_legacy_viewsdepends onformidable_admin, which is registered within_footer => true.
When the dependent is enqueued without the 5th argument, WordPress will silently move it to the footer, but only after an internal “group-upgrade” step. Being explicit avoids surprises and makes intent clearer to future maintainers.- wp_enqueue_script( - 'formidable_legacy_views', - FrmAppHelper::plugin_url() . '/js/admin/legacy-views.js', - array( 'jquery', 'formidable_admin' ), - FrmAppHelper::plugin_version() - ); +wp_enqueue_script( + 'formidable_legacy_views', + FrmAppHelper::plugin_url() . '/js/admin/legacy-views.js', + array( 'jquery', 'formidable_admin' ), + FrmAppHelper::plugin_version(), + true // load in footer – matches dependency group +);Check that nothing else on the page still relies on the script being printed in the header before applying.
js/admin/legacy-views.js (2)
118-120: Avoid implicit globalDOM
DOMis assigned withoutconst / let, polluting the global scope. Declare it explicitly:-DOM = tinymce.DOM; +const DOM = tinymce.DOM;
157-160: Overwritingwindow.onscrollcan clobber other listenersUsing
addEventListenerkeeps existing handlers intact and is the recommended modern pattern:-window.onscroll = document.documentElement.onscroll = setMenuOffset; +window.addEventListener( 'scroll', setMenuOffset ); +document.documentElement.addEventListener( 'scroll', setMenuOffset );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/controllers/FrmAppController.php(1 hunks)js/admin/legacy-views.js(1 hunks)js/formidable_admin.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
js/admin/legacy-views.js (2)
js/formidable_admin.js (5)
event(10479-10479)event(10642-10642)$advInfo(10137-10137)frmAdminJs(240-240)t(10085-10085)classes/controllers/FrmAppController.php (1)
menu(11-24)
classes/controllers/FrmAppController.php (1)
classes/helpers/FrmAppHelper.php (3)
FrmAppHelper(6-4568)plugin_url(65-68)plugin_version(44-46)
js/formidable_admin.js (1)
js/admin/legacy-views.js (1)
event(16-16)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Run PHP Syntax inspection (8.3)
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
🔇 Additional comments (1)
js/formidable_admin.js (1)
10640-10644: Legacy views code properly moved to custom event pattern 👍The change correctly moves the legacy views-specific code out of the main admin JS file by replacing a direct function call with a custom event. This is an excellent refactoring approach that keeps the functionality while properly separating concerns.
The dispatched custom event includes the necessary data for the legacy views script to handle this functionality elsewhere.
- toggleAllowedShortcodes( this.id ); + const event = new CustomEvent( 'frm_legacy_views_handle_field_focus' ); + event.frmData = { idAttrValue: this.id }; + document.dispatchEvent( event );
1 participant
This update moves over 400 lines of code out of the
frm_admin.js, loading it only when we're loading scripts for the legacy views editor now instead.This code won't likely get touched again. I may eventually move this to the Legacy Views add-on, but it isn't currently in my plans to update Legacy Views again.
Pre-release
formidable-beta.zip