PHP 8.4 compatibility, media library updates, and bug fixes#16
Merged
Conversation
…log/product prefixes
- Use explicit nullable type for $directoryResolver parameter - Reorder constructor parameters to place optional parameter after required ones Fixes deprecation warnings on Magento 2.4.8 with PHP 8.4. Resolves #13
…ount module as volume - Install Magento during Docker build instead of bootstrap script - Mount module source as volume at app/code/ImageKit/ImageKitMagento - Add Node.js 20 via nvm for frontend tooling - Update all script paths to new module location - Add script to set admin session timeout to 1 year - Simplify install-module.sh by removing rsync logic - Fix admin password default and improve permission handling
- Add ImportVideo controller to handle video imports with thumbnail generation - Implement retry logic for thumbnail retrieval with 202 status handling - Add getVideoImporterUrl method to Gallery Content block - Store video-thumbnail mappings in library_map table - Support video files in WYSIWYG upload controller - Update ImageKitImageProvider to handle full URLs for video thumbnails - Add video metadata to gallery JSON (provider, URL
- Remove trailing comma in Upload.php constructor (PHP 7.4 compatibility) - Replace @parse_url with parse_url in ImportVideo.php (error silencing)
- Switch to extdn/github-actions-m2/magento-phpstan/8.2@master for Composer 2 support - Update PHPCS job to PHP 8.2 with shivammathur/setup-php@2.36.0 - Remove composer self-update workaround (no longer needed with 8.2 image)
ahnv
changed the title
There was a problem hiding this comment.
Pull request overview
This PR enhances the ImageKit Magento extension to support PHP 8.4, MP4 video insertion into product galleries, and fixes image path handling to prevent duplicate catalog/product prefixes. It also updates the media library widget to version 2.4.1 and introduces a complete dev container setup for local Magento 2.4 development.
Changes:
- Fixed PHP 8.4 deprecation warnings by reordering constructor parameters and using explicit nullable types
- Normalized image path handling across three plugin files to prevent duplicate
catalog/productprefixes - Added MP4 video support with a new ImportVideo controller, frontend HTML5 player, and gallery plugin updates
- Updated imagekit-media-library-widget dependency from 1.0.4 to 2.4.1
- Added comprehensive dev container setup with Dockerfile, docker-compose, and bootstrap scripts
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Controller/Adminhtml/Cms/Wysiwyg/Images/Upload.php | Reordered constructor parameters to fix PHP 8.4 deprecation; added video file type handling |
| Controller/Adminhtml/Ajax/ImportVideo.php | New controller for importing MP4 videos from ImageKit with thumbnail download and retry logic |
| Core/ImageKitImageProvider.php | Refactored image path resolution into separate method; added support for absolute URLs |
| Plugin/Helper/Image.php | Normalized image paths to prevent duplicate catalog/product prefixes |
| Plugin/Catalog/Model/Product/Image/UrlBuilder.php | Normalized image paths to prevent duplicate catalog/product prefixes |
| Plugin/Catalog/Block/Product/ImageFactory.php | Normalized image paths to prevent duplicate catalog/product prefixes |
| Plugin/AddImagesToGalleryBlock.php | Added videoProvider injection into gallery JSON data |
| view/frontend/web/js/video-html5.js | HTML5 video player widget for ImageKit videos with iframe/srcdoc support |
| view/frontend/web/js/fotorama-add-video-events-mixin.js | Mixin to handle ImageKit video provider in Fotorama gallery |
| view/frontend/requirejs-config.js | RequireJS configuration for video modules |
| view/adminhtml/web/js/panel.js | Updated to use media library widget 2.4.1; added video importer URL routing |
| view/adminhtml/requirejs-config.js | Updated media library widget CDN version to 2.4.1 |
| Block/Adminhtml/Product/Helper/Form/Gallery/Content.php | Added getVideoImporterUrl method |
| etc/adminhtml/di.xml | Added video file extension configuration for CMS storage |
| etc/db_schema.xml | Added asset_type column to library map table |
| etc/csp_whitelist.xml | Added media-src policy for ImageKit domains |
| .devcontainer/* | Complete dev container setup for local Magento 2.4 development |
| .github/workflows/ci.yml | Updated GitHub Actions to use PHP 8.2 and latest action versions |
Comments suppressed due to low confidence (5)
view/adminhtml/web/js/panel.js:8
- Trailing comma in function parameter list should be removed.
], function (Element, $, ikMLWidget, uiAlert, notification, $t,) {
view/adminhtml/web/js/panel.js:11
- Missing semicolon at the end of the statement.
const IKMediaLibraryWidget = ikMLWidget.ImagekitMediaLibraryWidget
view/adminhtml/web/js/panel.js:48
- The video file extension check duplicates logic found in the Upload.php controller. Consider extracting this to a shared configuration or constant to maintain consistency across frontend and backend validation.
var isVideo = data.fileType && data.fileType !== 'image' &&
/\.(mp4|webm|ogv|mov)$/i.test(data.name);
Controller/Adminhtml/Ajax/ImportVideo.php:213
- Method name 'retrieveRemoteImage' is misleading since this method is used to download video thumbnails, not images. Consider renaming to 'retrieveRemoteThumbnail' or 'downloadVideoThumbnail'.
protected function retrieveRemoteImage($fileUrl, $localFilePath, $maxRetries = 5, $retryDelaySec = 3)
.github/workflows/ci.yml:26
- Using wildcard version constraint (*) for magento-coding-standard can lead to unpredictable behavior. Consider pinning to a specific version range (e.g., ^35) for consistent CI builds.
composer global require magento/magento-coding-standard:*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ix JS linting issues - Rename retrieveRemoteImage method to retrieveRemoteThumbnail for clarity - Remove trailing comma in panel.js function parameters - Add missing semicolon after IKMediaLibraryWidget constant declaration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses PHP 8.4 compatibility issues, fixes image path handling to prevent duplicate
catalog/productprefixes, updates the media library widget from 1.0.4 to 2.4.1, adds a dev container for local Magento 2.4 development, and fixes MP4 video insertion into product galleries.Changes
Details
PHP 8.4 Deprecation Fix (Resolves #13)
?DirectoryResolver) for optional$directoryResolverparameterbin/magento setup:di:compileon Magento 2.4.8 with PHP 8.4Image Path Handling (Resolves IK-2457)
catalog/productprefixesDependencies (Resolves #14)
imagekit-media-library-widgetfrom 1.0.4 to 2.4.1MP4 Video Insertion Fix (Resolves #15)
external-videomedia type withvideo_provider: imagekitDev Container