Fix bug with detecting section id when adding a field to a section#2484
Conversation
WalkthroughAdjusts drag-and-drop handling in frmAdminBuildJS to determine the destination section using ul.start_divider instead of ul.frm_sorting. Computes newSectionId via the nearest .edit_field_type_divider when present, adding a null-safe path. Other move, cleanup, and debounce behaviors remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as Admin UI (Drag/Drop)
participant JS as frmAdminBuildJS
participant DOM as DOM
U->>UI: Drag field
U->>UI: Drop over placeholder
UI->>JS: handleFieldDrop(placeholder, field)
JS->>DOM: placeholder.closest('ul.start_divider')
DOM-->>JS: newSection (nullable)
alt Section found
JS->>DOM: newSection.closest('.edit_field_type_divider')
DOM-->>JS: sectionContainer
JS->>JS: newSectionId = parseInt(sectionContainer.dataset.fid)
else No section
JS->>JS: newSectionId = 0
end
JS->>DOM: Move field into target position
JS->>DOM: Remove placeholder
JS-->>UI: Debounced sync trigger
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
js/src/admin/admin.js (1)
1001-1001: Nit: add radix to parseInt and use dataset/optional chaining for resilienceMinor hardening and clarity.
-const newSectionId = newSection ? parseInt( newSection.closest( '.edit_field_type_divider' ).getAttribute( 'data-fid' ) ) : 0; +const newSectionId = newSection + ? parseInt( newSection.closest( '.edit_field_type_divider' )?.dataset?.fid ?? '0', 10 ) + : 0;You could mirror the radix addition for previousSectionId as well (Line 1000).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
js/src/admin/admin.js(2 hunks)
⏰ 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). (3)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (1)
js/src/admin/admin.js (1)
992-992: Correct fix: reliably resolve destination section via ul.start_dividerUsing placeholder.closest('ul.start_divider') and a null-safe newSectionId eliminates mis-detection when dropping into a row inside a section (vs top-level/form group). This should resolve the bug where moves into sections reported section_id 0.
Also applies to: 1001-1001
No description provided.