Revert "fix: Add banner image dimension settings reflection on admin vendor page." - #3042
Conversation
…vendor p…" This reverts commit 1cb516f.
WalkthroughThe changes refactor image cropping logic across multiple files, introducing dynamic computation of crop dimensions based on actual image aspect ratios. A header image model now determines cropping needs, and theme dimensions are adjusted to fit computed aspect ratios. The JavaScript cropper initialization is updated to use Cropper with a fixed large suggested width, while template styling removes implicit minimum sizing constraints for banner elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assets/src/js/script.jssrc/admin/dashboard/pages/vendor-create-edit/WpMedia.tssrc/admin/dashboard/pages/vendor-create-edit/form/General.tsxtemplates/settings/store-form.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
src/admin/dashboard/pages/vendor-create-edit/form/General.tsx
🧬 Code graph analysis (3)
src/admin/dashboard/pages/vendor-create-edit/WpMedia.ts (1)
assets/src/js/product-editor.js (3)
xInit(1293-1300)realHeight(1292-1292)realWidth(1291-1291)
src/admin/dashboard/pages/vendor-create-edit/form/General.tsx (1)
src/admin/dashboard/pages/vendor-create-edit/WpMedia.ts (1)
wpMedia(52-271)
assets/src/js/script.js (1)
assets/src/js/product-editor.js (4)
realHeight(1292-1292)realWidth(1291-1291)api(1304-1304)xInit(1293-1300)
⏰ 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). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (8)
templates/settings/store-form.php (1)
276-283: LGTM - Banner styling reverted to use max constraints.The styling changes appropriately use
max-widthandmax-heightwith the actual banner dimensions from settings, rather than minimum fallbacks. This provides proper responsive behavior while respecting configured dimensions.src/admin/dashboard/pages/vendor-create-edit/form/General.tsx (1)
319-321: Banner upload uses default dimensions from settings.Calling
wpMedia(setBanner)without explicit dimensions will fall back todokanAdminDashboard.store_banner_dimensionvalues inWpMedia.ts. This is acceptable for the revert.assets/src/js/script.js (2)
329-332: Cropper controller with large suggestedWidth.Changed from
CustomizeImageCroppertoCropperwithsuggestedWidth: 5000. The large suggested width effectively allows any width, deferring actual constraints toimgSelectOptions.
192-202: Aspect ratio calculation adjusts crop region to fit image.The logic correctly scales
xInit/yInitto maintain the target aspect ratio while fitting within the actual image dimensions. This ensures the initial crop selection doesn't exceed image bounds.src/admin/dashboard/pages/vendor-create-edit/WpMedia.ts (4)
57-76: Dimension calculation logic mirrors script.js for consistency.The pattern of using mutable
xInit/yInit, extracting real dimensions, and computing aspect-ratio-constrained crop regions is consistent withassets/src/js/script.js. This ensures uniform cropping behavior across frontend and admin contexts.
92-102: Aspect ratio adjustment correctly constrains crop to image bounds.The conditional logic correctly determines whether to scale by width or height based on comparing image aspect ratio to target ratio, ensuring the crop region fits within the actual image while maintaining the desired proportions.
117-125: Conditional constraint application based on flex settings.The flex flag checks correctly apply
aspectRatioonly when both dimensions are fixed, andmaxHeight/maxWidthwhen the respective dimension is non-flexible. This matches the expected behavior for WordPress media cropping.
216-222: Different cropper controllers used intentionally for respective contexts.WpMedia.ts correctly uses
wp.media.controller.CustomizeImageCropperwith thecontrolparameter in the admin dashboard component, while script.js appropriately uses the basewp.media.controller.Cropperclass for vendor frontend settings. This difference aligns with WordPress media API design where CustomizeImageCropper extends Cropper to handle Customizer-specific functionality.
| this.headerImage = new api.HeaderTool.ImageModel(); | ||
| this.headerImage.set({ | ||
| themeWidth: xInit, | ||
| themeHeight: yInit, | ||
| themeFlexWidth: flexWidth, | ||
| themeFlexHeight: flexHeight, | ||
| imageWidth: realWidth, | ||
| imageHeight: realHeight | ||
| }); | ||
|
|
||
| controller.set('canSkipCrop', !this.headerImage.shouldBeCropped()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the file and examine its context
git ls-files | grep -E "script\.js|customize" | head -20Repository: getdokan/dokan
Length of output: 321
🏁 Script executed:
#!/bin/bash
# First, find the file
fd "script\.js" --type f
# Then examine its contents around lines 180-190
echo "=== Searching for api/wp.customize references ==="
rg "wp\.customize|api\.HeaderTool|var api" -B2 -A2 --type=js assets/Repository: getdokan/dokan
Length of output: 3061
🏁 Script executed:
#!/bin/bash
# Check the file header and how it's loaded
head -n 30 assets/src/js/script.js
echo -e "\n=== Lines around 180-190 ==="
sed -n '170,200p' assets/src/js/script.js
echo -e "\n=== Search for api variable definition ==="
rg "var api|const api|let api|api\s*=" assets/src/js/script.js -B2 -A2Repository: getdokan/dokan
Length of output: 1921
🏁 Script executed:
#!/bin/bash
# Search for where script.js is enqueued
rg "script\.js|dokan-script" --type=php -B2 -A2 | head -100
echo -e "\n=== Search for enqueue_scripts in Dokan files ==="
fd "\.php$" | xargs rg "wp_enqueue_script|admin_enqueue_scripts|wp_enqueue_scripts" | grep -i "script.js\|dokan.*script" | head -20Repository: getdokan/dokan
Length of output: 2232
🏁 Script executed:
#!/bin/bash
# Look for the main Dokan class that loads scripts
fd "class-.*\.php" | xargs grep -l "enqueue" | head -5
# Check the assets or main plugin file
cat dokan.php | head -100Repository: getdokan/dokan
Length of output: 2752
🏁 Script executed:
#!/bin/bash
# Look at the load_dokan_global_scripts function
sed -n '1,500p' includes/Assets.php | grep -A 50 "function load_dokan_global_scripts"
echo -e "\n=== Check full context of dokan-script enqueue ==="
rg "dokan-script.*=>|load_dokan_global_scripts" includes/Assets.php -B5 -A15 | head -100Repository: getdokan/dokan
Length of output: 3246
🏁 Script executed:
#!/bin/bash
# Search for all places where dokan-script is enqueued
rg "wp_enqueue_script\(\s*['\"]dokan-script" includes/Assets.php -B3 -A3
echo -e "\n=== Check the context around those enqueues ==="
rg "enqueue_front_scripts|enqueue_admin_scripts" includes/Assets.php -A30 | head -150Repository: getdokan/dokan
Length of output: 6106
🏁 Script executed:
#!/bin/bash
# Check what customize-base and customize-model are
rg "customize-base|customize-model" --type=php includes/Assets.php
echo -e "\n=== Check the full enqueue_front_scripts function ==="
sed -n '/public function enqueue_front_scripts/,/^ public function/p' includes/Assets.php | head -200Repository: getdokan/dokan
Length of output: 8718
🏁 Script executed:
#!/bin/bash
# Let me verify: is there actually a guard at the start of script.js file?
head -10 assets/src/js/script.js
echo -e "\n=== Check if there's a guard before api usage ==="
sed -n '1,200p' assets/src/js/script.js | grep -B5 -A5 "api\.HeaderTool"Repository: getdokan/dokan
Length of output: 676
🏁 Script executed:
#!/bin/bash
# Let me check the dependencies more carefully
# Is wp.customize guaranteed to be available when customize-base/model are loaded?
rg "customize-base|customize-model" --type=php -B2 -A2
echo -e "\n=== Search for where customize-base/model are registered ==="
rg "wp-includes/js/customize" includes/Assets.php -B2 -A2
echo -e "\n=== Check all usages of api.HeaderTool in script.js ==="
rg "api\." assets/src/js/script.js | head -20Repository: getdokan/dokan
Length of output: 1919
🏁 Script executed:
#!/bin/bash
# Confirm the actual filename - is it dokan.js or script.js?
ls -la assets/src/js/*.js | grep -E "dokan|script"
echo -e "\n=== Check line 2 of the actual file used (dokan.js) ==="
head -5 assets/src/js/dokan.js
echo -e "\n=== Verify the structure around lines 180-190 of dokan.js ==="
sed -n '175,200p' assets/src/js/dokan.js
echo -e "\n=== Check if dokan.js is compiled from script.js or is script.js the compiled version ==="
head -20 assets/src/js/script.js
head -20 assets/src/js/dokan.jsRepository: getdokan/dokan
Length of output: 1423
🏁 Script executed:
#!/bin/bash
# Search for the context of where headerImage is used - what function/object contains it
rg "this\.headerImage" assets/src/js/script.js -B20 | head -80
echo -e "\n=== Look for product-editor.js which had a guard ==="
rg "typeof wp|wp\.customize" assets/src/js/product-editor.js -B2 -A2Repository: getdokan/dokan
Length of output: 2283
Guard wp.customize access before using HeaderTool.ImageModel().
The code assigns var api = wp.customize; at line 2 without checking if wp.customize exists. The wp.customize API is only available in the WordPress Customizer context, but this script is enqueued on public-facing pages (store, product, account) via enqueue_front_scripts() where it will be undefined, causing runtime errors when calculateImageSelectOptions() and calculateImageSelectOptionsProfile() call api.HeaderTool.ImageModel().
Add a guard similar to the one in product-editor.js:
if ( typeof wp !== 'undefined' && wp.customize ) {
var api = wp.customize;
// ... use api.HeaderTool
}Or wrap the methods that use api with conditional checks before instantiating or calling HeaderTool.
| <span className="text-neutral-400 -mt-1"> | ||
| { sprintf( | ||
| /* translators: 1) banner width, 2) banner height. */ | ||
| __( 'Size - %1$s X %2$s px' ), | ||
| bannerWidth, | ||
| bannerHeight | ||
| { __( | ||
| 'Size - 600 X 325 px', | ||
| 'dokan-lite' | ||
| ) } | ||
| </span> |
There was a problem hiding this comment.
Hardcoded banner size may not match actual settings.
The banner size text is hardcoded to "600 X 325 px", but the wpMedia function (called without dimension arguments) will use dokanAdminDashboard.store_banner_dimension for cropping. If admin settings differ from 600×325, the displayed hint won't match actual cropping behavior.
Consider using the dimension values from the dashboard settings for consistency:
{ `Size - ${dokanAdminDashboard.store_banner_dimension.width} X ${dokanAdminDashboard.store_banner_dimension.height} px` }🤖 Prompt for AI Agents
In src/admin/dashboard/pages/vendor-create-edit/form/General.tsx around lines
306 to 311, the banner size hint is hardcoded to "600 X 325 px" but wpMedia uses
dokanAdminDashboard.store_banner_dimension for cropping; replace the hardcoded
text with a dynamic string built from
dokanAdminDashboard.store_banner_dimension.width and .height (safely handling
missing values with sensible defaults) so the displayed hint matches actual
settings and continue to pass the resulting string through the localization
function.
2 participants
Reverts #3019
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.