fix: design perfection and link for post form custom field modal #1583
Conversation
WalkthroughThe updates modify the appearance of SweetAlert modal titles by enforcing a black color and enhance the form builder's tooltip interactions. In the form builder, tooltips now include interactive links for navigation and improved image styling, with additional logic for opening and scrolling to specific settings panels. Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the custom-field informational popup by making the “Advanced Options” and “Show data on post” texts clickable so they reliably open and expand the corresponding accordion in the field settings panel. It also ensures the accordion is fully rendered before toggling and scrolls it into view, and adds a title color override.
- Converted popup instructions into interactive links that dispatch a Vuex action to open the correct field’s settings.
- Introduced
setTimeout+Vue.nextTicksequencing to wait for Vue’s DOM render before finding and toggling the “Advanced Options” accordion. - Added style overrides for the SweetAlert2 title color in both LESS and compiled CSS.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| assets/less/admin.less | Added color: #000000 !important; to the SweetAlert2 title. |
| assets/css/admin.css | Added color: #000000 !important; to the SweetAlert2 title (duplicated in compiled CSS). |
| assets/js/wpuf-form-builder.js | Updated popup HTML to use <a> links; added click handler that commits Vuex, waits for render, toggles the accordion, and scrolls it into view. |
Comments suppressed due to low confidence (1)
assets/js/wpuf-form-builder.js:198
- This new click handler and timing logic are not covered by any tests; consider adding unit or integration tests to validate that the accordion always opens and scrolls under different render timings.
$(modal).find('a.wpuf-swal-action-link[data-action="open-advanced-options"]').on('click', function(e) {
| @@ -1,5 +1,6 @@ | |||
| h2.swal2-title { | |||
| line-height: 1.2em; | |||
| color: #000000 !important; | |||
There was a problem hiding this comment.
You’ve added the same style override in both the LESS source and the compiled CSS; maintain a single source of truth in admin.less and recompile to avoid duplication.
| color: #000000 !important; | |
| color: #000000 !important; /* Ensure this rule is only in LESS */ |
Copilot uses AI. Check for mistakes.
| @@ -1,5 +1,6 @@ | |||
| h2.swal2-title { | |||
| line-height: 1.2em; | |||
| color: #000000 !important; | |||
There was a problem hiding this comment.
This override is duplicated from admin.less; consider removing it here and generating it via your build process to keep styles in sync.
| color: #000000 !important; |
Copilot uses AI. Check for mistakes.
assets/js/wpuf-form-builder.js
Outdated
| console.log('WPUF Debug: Swal link clicked for field ID:', fieldId); | ||
|
|
||
| Swal.close(); | ||
|
|
||
| setTimeout(() => { | ||
| wpuf_form_builder_store.commit('open_field_settings', fieldId); | ||
| console.log('WPUF Debug: open_field_settings committed.'); | ||
|
|
||
| Vue.nextTick(() => { | ||
| console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.'); |
There was a problem hiding this comment.
[nitpick] Remove or wrap these console logging statements so they don’t clutter the console in production environments.
| console.log('WPUF Debug: Swal link clicked for field ID:', fieldId); | |
| Swal.close(); | |
| setTimeout(() => { | |
| wpuf_form_builder_store.commit('open_field_settings', fieldId); | |
| console.log('WPUF Debug: open_field_settings committed.'); | |
| Vue.nextTick(() => { | |
| console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.'); | |
| if (isDebugMode) { | |
| console.log('WPUF Debug: Swal link clicked for field ID:', fieldId); | |
| } | |
| Swal.close(); | |
| setTimeout(() => { | |
| wpuf_form_builder_store.commit('open_field_settings', fieldId); | |
| if (isDebugMode) { | |
| console.log('WPUF Debug: open_field_settings committed.'); | |
| } | |
| Vue.nextTick(() => { | |
| if (isDebugMode) { | |
| console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.'); | |
| } |
Copilot uses AI. Check for mistakes.
assets/js/wpuf-form-builder.js
Outdated
| html += sprintf( '<p class="wpuf-text-base">%s<span class="wpuf-text-primary">%s</span>%s<span class="wpuf-text-primary">%s</span>%s</p>', __( 'Edit the custom field inside the post form and on the right side you will see', 'wp-user-frontend' ), __( '"Advanced Options".', 'wp-user-frontend' ), __( ' Expand that, scroll down and you will see ', 'wp-user-frontend' ), __( '"Show data on post"', 'wp-user-frontend' ), __( ' - set this yes.', 'wp-user-frontend' ) ); | ||
| html += '<img src="' + image_two + '" alt="custom field data">'; | ||
| var fieldIdForLink = payload.field.id; | ||
| html += sprintf( '<p class="wpuf-text-base">%s<a href="#" class="wpuf-text-primary wpuf-swal-action-link wpuf-font-bold" data-action="open-advanced-options" data-field-id="%s">%s</a>%s<a href="#" class="wpuf-text-primary wpuf-swal-action-link wpuf-font-bold" data-action="open-advanced-options" data-field-id="%s">%s</a>%s</p>', |
There was a problem hiding this comment.
[nitpick] Using <a href="#"> for actions can cause unexpected navigation; consider using a <button> or adding role="button" and an aria-label for improved accessibility.
Copilot uses AI. Check for mistakes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/css/admin.css(1 hunks)assets/js/wpuf-form-builder.js(2 hunks)assets/less/admin.less(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
assets/js/wpuf-form-builder.js
[error] 263-263: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 265-265: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 276-276: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run PHPCS inspection
- GitHub Check: test
🔇 Additional comments (3)
assets/css/admin.css (1)
1-4: LGTM!The CSS change properly sets the SweetAlert2 modal title color to black, ensuring consistent visibility across different themes.
assets/less/admin.less (1)
1-4: LGTM!The LESS change correctly mirrors the CSS update, maintaining consistency between source and compiled files.
assets/js/wpuf-form-builder.js (1)
168-196: Well-implemented interactive tooltip enhancements!The conversion of static instructions to clickable links improves user experience by allowing direct navigation to relevant settings. The rounded corners on images and updated button color enhance visual consistency.
assets/js/wpuf-form-builder.js
Outdated
|
|
||
| Swal.close(); | ||
|
|
||
| setTimeout(() => { | ||
| wpuf_form_builder_store.commit('open_field_settings', fieldId); | ||
| console.log('WPUF Debug: open_field_settings committed.'); | ||
|
|
||
| Vue.nextTick(() => { | ||
| console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.'); | ||
|
|
||
| setTimeout(() => { // Single timeout after Vue.nextTick | ||
| console.log('WPUF Debug: Inner setTimeout fired, attempting to find and toggle panel elements.'); | ||
|
|
||
| let advancedOptionsTargetText = ''; | ||
| if (typeof wpuf_form_builder_mixins !== 'undefined' && | ||
| typeof wpuf_form_builder_mixins(Vue.prototype).i18n !== 'undefined' && | ||
| wpuf_form_builder_mixins(Vue.prototype).i18n.advanced_options) { | ||
| advancedOptionsTargetText = wpuf_form_builder_mixins(Vue.prototype).i18n.advanced_options; | ||
| } else { | ||
| advancedOptionsTargetText = __('"Advanced Options".', 'wp-user-frontend'); | ||
| } | ||
| advancedOptionsTargetText = advancedOptionsTargetText.replace(/"/g, '').replace(/\.$/, "").trim().toLowerCase(); | ||
| console.log('WPUF Debug: Target Adv. Opt. Text (Normalized):', advancedOptionsTargetText); | ||
|
|
||
| var $fieldOptionsMainContainer = $('div.wpuf-form-builder-field-options'); | ||
|
|
||
| if (!$fieldOptionsMainContainer.length) { | ||
| console.warn('WPUF Form Builder Debug: Field options main container "div.wpuf-form-builder-field-options" NOT FOUND after delay.'); | ||
| return; | ||
| } | ||
| // Check if the loader/placeholder is still visible | ||
| if ($fieldOptionsMainContainer.find('> div:first-child[class*="text-center"]').is(':visible') && $fieldOptionsMainContainer.find('.option-fields-section').length === 0) { | ||
| console.warn('WPUF Form Builder Debug: Loader/placeholder seems to be still visible or settings sections not rendered in .wpuf-form-builder-field-options. Action aborted.'); | ||
| return; | ||
| } | ||
|
|
||
| console.log('WPUF Debug: Field options main container "div.wpuf-form-builder-field-options" found:', $fieldOptionsMainContainer[0]); | ||
|
|
||
| var $advancedOptionsToggle, $advancedOptionsContentDiv, $sectionToScroll; | ||
| var $sections = $fieldOptionsMainContainer.find('.option-fields-section'); | ||
| console.log('WPUF Debug: Found ' + $sections.length + ' .option-fields-section elements within the main container.'); | ||
|
|
||
| $sections.each(function(index) { | ||
| var $parentSection = $(this); | ||
| var $h3 = $parentSection.find('h3').first(); | ||
| if (!$h3.length) return true; | ||
|
|
||
| var rawH3Text = $h3.clone().children('i').remove().end().text(); | ||
| var normalizedH3Text = rawH3Text.trim().toLowerCase().replace(/\.$/, ""); | ||
|
|
||
| console.log('WPUF Debug: (Section ' + index + ') H3 raw: "' + rawH3Text + '", Normalized: "' + normalizedH3Text + '"'); | ||
|
|
||
| if (normalizedH3Text === advancedOptionsTargetText) { | ||
| $advancedOptionsToggle = $h3; | ||
| $advancedOptionsContentDiv = $parentSection.find('div.option-field-section-fields').first(); | ||
| $sectionToScroll = $parentSection; | ||
| console.log('WPUF Debug: Matched h3 for "Advanced Options":', $advancedOptionsToggle[0]); | ||
| return false; | ||
| } | ||
| }); | ||
|
|
||
| if ($advancedOptionsToggle && $advancedOptionsToggle.length) { | ||
| var isContentVisible = false; | ||
| if ($advancedOptionsContentDiv && $advancedOptionsContentDiv.length) { | ||
| isContentVisible = $advancedOptionsContentDiv.is(':visible'); | ||
| console.log('WPUF Debug: Advanced Options content visible before click?', isContentVisible); | ||
| } else { | ||
| console.warn('WPUF Form Builder Debug: Advanced options content div (.option-field-section-fields) not found relative to matched h3.'); | ||
| } | ||
|
|
||
| if (!isContentVisible) { | ||
| console.log('WPUF Debug: Attempting to click "Advanced Options" h3 toggle.'); | ||
| $advancedOptionsToggle.trigger('click'); | ||
| setTimeout(() => { | ||
| if ($advancedOptionsContentDiv && $advancedOptionsContentDiv.length) { | ||
| console.log('WPUF Debug: Content visible after click and 150ms delay?', $advancedOptionsContentDiv.is(':visible')); | ||
| } | ||
| }, 150); | ||
| } else { | ||
| console.log('WPUF Debug: "Advanced Options" content already visible.'); | ||
| } | ||
|
|
||
| if (!$sectionToScroll || !$sectionToScroll.length) $sectionToScroll = $advancedOptionsToggle; | ||
|
|
||
| setTimeout(() => { | ||
| const elementToScrollTo = $sectionToScroll.get(0); | ||
| if (elementToScrollTo && typeof elementToScrollTo.scrollIntoView === 'function') { | ||
| elementToScrollTo.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'start' }); | ||
| console.log('WPUF Debug: Scrolled "Advanced Options" section into view.'); | ||
| } | ||
| }, 350); | ||
| } else { | ||
| console.warn('WPUF Form Builder Debug: Could not find "Advanced Options" h3 toggle based on text: "' + advancedOptionsTargetText + '".'); | ||
| } | ||
| }, 400); // Inner timeout duration (e.g., 400ms) | ||
|
|
||
| }); | ||
| }, 250); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider production-ready improvements for the advanced options toggle logic.
While the implementation is thorough, consider the following improvements:
- Debug logging: The extensive console.log statements should be removed or wrapped in a debug flag for production builds.
- Timing reliability: The nested timeouts with fixed delays (250ms, 400ms, 150ms, 350ms) may fail on slower systems. Consider using a more robust approach like mutation observers or retry logic with exponential backoff.
- didOpen: (modal) => {
+ didOpen: (modal) => {
+ const DEBUG = false; // Set to true during development
$(modal).find('a.wpuf-swal-action-link[data-action="open-advanced-options"]').on('click', function(e) {
e.preventDefault();
var fieldId = $(this).data('field-id');
- console.log('WPUF Debug: Swal link clicked for field ID:', fieldId);
+ if (DEBUG) console.log('WPUF Debug: Swal link clicked for field ID:', fieldId);
Swal.close();
- setTimeout(() => {
+ // Use a more robust approach with retry logic
+ const waitForFieldOptions = (attempts = 0, maxAttempts = 5) => {
wpuf_form_builder_store.commit('open_field_settings', fieldId);
- console.log('WPUF Debug: open_field_settings committed.');
+ if (DEBUG) console.log('WPUF Debug: open_field_settings committed.');
Vue.nextTick(() => {
- console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.');
+ if (DEBUG) console.log('WPUF Debug: Vue.nextTick for toggling advanced options triggered.');
setTimeout(() => { // Single timeout after Vue.nextTick
- console.log('WPUF Debug: Inner setTimeout fired, attempting to find and toggle panel elements.');
+ if (DEBUG) console.log('WPUF Debug: Inner setTimeout fired, attempting to find and toggle panel elements.');Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 263-263: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 265-265: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 276-276: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In assets/js/wpuf-form-builder.js around lines 197 to 301, remove or
conditionally wrap all console.log and console.warn debug statements behind a
debug flag to avoid cluttering production logs. Replace the nested fixed-delay
setTimeout calls with a more reliable mechanism such as MutationObserver to
detect DOM changes or implement retry logic with exponential backoff to ensure
elements are ready before proceeding, improving timing reliability on slower
systems.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
assets/js/wpuf-form-builder.js (1)
172-183: Address accessibility concern for link elements.The use of
href="#"for action links was previously flagged for accessibility concerns. Consider using<button>elements or adding properrole="button"andaria-labelattributes.
🧹 Nitpick comments (1)
assets/js/wpuf-form-builder.js (1)
215-295: Consider simplifying the text matching and DOM manipulation logic.The current approach of normalizing and matching text content for "Advanced Options" could be fragile if the UI text changes. Consider these improvements:
- Use data attributes: Add
data-section="advanced-options"to the relevant DOM elements during rendering- Centralize selectors: Define reusable CSS selectors instead of complex jQuery traversal
- Extract helper functions: Break down the complex logic into smaller, testable functions
Example approach:
// Helper function for finding sections by data attribute function findAdvancedOptionsSection() { return $('.wpuf-form-builder-field-options [data-section="advanced-options"]'); } // Helper function for toggling section function toggleAdvancedOptions($section) { const $toggle = $section.find('h3').first(); const $content = $section.find('.option-field-section-fields').first(); if (!$content.is(':visible')) { $toggle.trigger('click'); } return $content; }This approach would be more maintainable and less dependent on internationalized text content.
🧰 Tools
🪛 Biome (1.9.4)
[error] 263-263: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 265-265: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 276-276: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/wpuf-form-builder.js(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
assets/js/wpuf-form-builder.js
[error] 263-263: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 265-265: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 276-276: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Run PHPCS inspection
🔇 Additional comments (3)
assets/js/wpuf-form-builder.js (3)
168-169: LGTM: Proper link construction and styling.The HTML construction correctly creates a link to the settings page with appropriate target and internationalization.
196-196: LGTM: Simple color adjustment.The cancel button color change is a minor styling improvement.
1283-1283: LGTM: Removed unnecessary console logging.Good practice to remove console logging from production code, especially in error handlers.
Subject: Fix: Ensure "Advanced Options" opens reliably from custom field pop-up
Description:
This update addresses an issue where clicking the "Advanced Options" or "Show data on post" links within the informational pop-up for new custom fields did not consistently navigate to and expand the corresponding "Advanced Options" accordion in the field settings panel.
The primary cause was a timing problem: the JavaScript code attempting to find and interact with the "Advanced Options" toggle was executing before the field settings panel (and its child elements) was fully rendered in the DOM by Vue.js after the panel switch was initiated.
The fix involves the following improvements:
open_field_settings) is dispatched to switch to the correct field's settings panel.setTimeoutis used in conjunction withVue.nextTickto ensure that operations on the DOM (like finding and clicking the "Advanced Options" toggle) occur only after Vue has processed state changes and the browser has had sufficient time to render the field options panel.div.wpuf-form-builder-field-options) rather than relying on a previously assumed ID.<h3>element serving as the "Advanced Options" toggle by matching its normalized text content (sourced fromi18nor__())..option-field-section-fields) is checked before attempting a click.<h3>element.This revised approach provides a more robust and reliable mechanism for guiding the user to the advanced settings of a newly added custom field directly from the informational pop-up.
Summary by CodeRabbit
New Features
Style