-
Notifications
You must be signed in to change notification settings - Fork 215
Optimize vendor selection with AJAX in quick/bulk edit to improve performance and UX #2807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Updated the vendor selection field in the WooCommerce product quick edit to use a dynamic select input with AJAX search, improving scalability and user experience. Exposed DokanAdminProduct to the global window object for use in inline edit scripts. Minor code cleanup and improved event handling for inline edit actions.
WalkthroughThe changes expose the Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Browser as Browser (JS)
participant Server as Server (PHP/AJAX)
Admin->>Browser: Opens Quick Edit for Product
Browser->>Browser: Inline edit form appears
Browser->>Browser: window.DokanAdminProduct.searchVendors() initializes AJAX search
Admin->>Browser: Types in vendor field
Browser->>Server: AJAX request for vendor search
Server-->>Browser: Returns matching vendors
Browser->>Admin: Displays searchable vendor dropdown
Estimated code review effort2 (~15 minutes) Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
assets/src/js/dokan-admin-product.js (1)
89-89
: Consider the implications of global exposure.Exposing
DokanAdminProduct
globally enables external access to its methods, which aligns with the PR's goal of enabling AJAX vendor selection in quick edit. However, this creates a global dependency that could lead to namespace pollution.Consider these alternatives:
- Use a more specific namespace like
window.Dokan = window.Dokan || {}; window.Dokan.AdminProduct = DokanAdminProduct;
- Only expose specific methods that need external access
- Document this global API for maintainability
- window.DokanAdminProduct = DokanAdminProduct; + // Expose DokanAdminProduct globally for external scripts (e.g., quick edit) + window.Dokan = window.Dokan || {}; + window.Dokan.AdminProduct = DokanAdminProduct;includes/wc-template.php (1)
183-193
: Improved event handling with potential timing issue.The use of delegated event binding and setTimeout is a good approach for dynamically created elements. However, the 100ms delay might not be sufficient in all cases.
Consider using a more robust approach to ensure the element is ready:
- setTimeout(function() { - const element = $(selector).find('.dokan_product_author_override_quick'); - if( ! $(element).hasClass('select2-hidden-accessible')) { - window.DokanAdminProduct.searchVendors(element); - } - }, 100) + // Poll until element is ready or timeout + let attempts = 0; + const maxAttempts = 50; // 5 seconds max + const checkElement = function() { + const element = $(selector).find('.dokan_product_author_override_quick'); + if (element.length && !element.hasClass('select2-hidden-accessible')) { + window.DokanAdminProduct.searchVendors(element); + } else if (attempts < maxAttempts) { + attempts++; + setTimeout(checkElement, 100); + } + }; + checkElement();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/src/js/dokan-admin-product.js
(1 hunks)includes/wc-template.php
(2 hunks)
🔇 Additional comments (4)
includes/wc-template.php (4)
40-40
: Minor text change looks good.The text change to "Vendor Info" improves clarity and consistency.
158-158
: Function signature change is well-designed.Adding the optional
$scope
parameter allows differentiation between quick and bulk edit contexts while maintaining backward compatibility.
199-208
: Action hook implementation is well-structured.The separation of quick edit and bulk edit with different scope parameters is a clean approach that enables proper differentiation of behavior.
169-176
: Security validation insearch_vendors
confirmedThe AJAX handler in includes/Admin/Hooks.php already enforces:
current_user_can( 'manage_woocommerce' )
capability check- Nonce verification via
wp_verify_nonce( sanitize_key( $_GET['_wpnonce'] ), 'dokan_admin_product' )
- Sanitization of the search input (
sanitize_text_field
+wp_unslash
)No further action needed.
setTimeout(function() { | ||
const element = $(selector).find('.dokan_product_author_override_quick'); | ||
if( ! $(element).hasClass('select2-hidden-accessible')) { | ||
window.DokanAdminProduct.searchVendors(element); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Verify global DokanAdminProduct availability.
The code assumes window.DokanAdminProduct
is always available. Add a safety check to prevent errors if the script hasn't loaded yet.
- if( ! $(element).hasClass('select2-hidden-accessible')) {
- window.DokanAdminProduct.searchVendors(element);
- }
+ if( ! $(element).hasClass('select2-hidden-accessible')) {
+ if (window.DokanAdminProduct && typeof window.DokanAdminProduct.searchVendors === 'function') {
+ window.DokanAdminProduct.searchVendors(element);
+ } else {
+ console.warn('DokanAdminProduct not available for vendor search');
+ }
+ }
🤖 Prompt for AI Agents
In includes/wc-template.php at line 190, the code calls
window.DokanAdminProduct.searchVendors without checking if
window.DokanAdminProduct exists. Add a conditional check to verify
window.DokanAdminProduct is defined before calling searchVendors to avoid
runtime errors if the script is not loaded.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit