Add missing thumbnails for new applications#2030
Conversation
WalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant Template
User->>Application: Request Application Data
Application->>Template: Fetch Template Keys
Template->>Template: get_template_keys_with_local_png_images()
Template->>Template: get_template_keys_with_local_webp_images()
Template-->>Application: Return Template Keys
Application->>User: Display Application with Images
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🧹 Outside diff range and nitpick comments (3)
js/src/form-templates/elements/applicationTemplatesElement.js (1)
63-66: Approve the addition of webp support with a minor optimization suggestion.The changes successfully implement the new logic for conditionally selecting between webp and png formats for thumbnails, aligning with the PR objectives. The implementation is correct and maintains backwards compatibility with the placeholder SVG fallback.
Consider optimizing the thumbnail URL selection logic for better readability:
-const thumbnailURL = hasLiteThumbnail ? - ( isWebp ? `${thumbnailBaseURL}/${key}.webp` : `${thumbnailBaseURL}/${key}.png` ) : - `${thumbnailBaseURL}/placeholder.svg`; +const thumbnailURL = hasLiteThumbnail + ? `${thumbnailBaseURL}/${key}.${isWebp ? 'webp' : 'png'}` + : `${thumbnailBaseURL}/placeholder.svg`;This refactoring maintains the same logic while making it slightly more concise and easier to read.
classes/models/FrmApplicationTemplate.php (2)
54-54: Update the@sincetag with the correct version numberIn the docblock for
get_template_keys_with_local_webp_images(), the@since x.xtag should be updated to reflect the actual version number when this method was introduced.Apply this diff to update the version number:
/** * Newer templates now use .webp files instead of .png. * - * @since x.x + * @since 6.0 * * @return array<string> */
79-85: Consider sorting the template keys alphabeticallyFor better readability and maintainability, consider sorting the array of template keys alphabetically.
Apply this diff to sort the array:
return array( + 'business-directory', 'business-hours', - 'business-directory', 'faq-template-wordpress', 'product-review', 'real-estate-listings', 'restaurant-menu', 'team-directory', );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
images/applications/thumbnails/business-directory.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (3)
- classes/models/FrmApplicationTemplate.php (2 hunks)
- js/admin/applications.js (1 hunks)
- js/src/form-templates/elements/applicationTemplatesElement.js (1 hunks)
🔇 Additional comments (5)
js/src/form-templates/elements/applicationTemplatesElement.js (2)
Line range hint
1-108: Overall implementation aligns well with PR objectives.The changes successfully introduce support for webp thumbnails while maintaining backwards compatibility. The code is well-structured and follows good practices. The minor suggestions provided should help improve readability and maintainability.
Line range hint
23-26: Approve the change to returnundefinedinstead ofnull.The modification to return
undefinedinstead ofnullwhen no applications are provided aligns with JavaScript best practices. This change enhances code clarity and consistency.To ensure this change doesn't introduce unexpected behavior, please run the following script to check for any code that might be relying on the previous
nullreturn value:✅ Verification successful
Verification Complete: No dependencies found on
nullreturned fromcreateApplicationTemplates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for potential usage of createApplicationTemplates that might expect null # Test: Search for createApplicationTemplates usage rg --type javascript -A 5 'createApplicationTemplates\s*\(' | grep -E '(===|!==)\s*null'Length of output: 125
Script:
#!/bin/bash # Description: Search for potential usage of createApplicationTemplates that might expect null # Corrected Test: Search for createApplicationTemplates usage in JavaScript files rg --type js -A 5 'createApplicationTemplates\s*\(' | grep -E '(===|!==)\s*null'Length of output: 82
classes/models/FrmApplicationTemplate.php (3)
44-47: LGTM!The merging of PNG and WEBP image keys into
self::$keys_with_imagesis correctly implemented.
181-181: LGTM!The addition of the
isWebpflag correctly identifies applications using WEBP images.
77-77: Ensure all references to the renamed method are updatedThe method
get_template_keys_with_local_images()has been renamed toget_template_keys_with_local_png_images(). Please verify that all references to the old method name have been updated throughout the codebase.Run the following script to check for any occurrences of
get_template_keys_with_local_images():✅ Verification successful
All references to
get_template_keys_with_local_imageshave been successfully updated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old method name in PHP files. # Test: Find any occurrences of 'get_template_keys_with_local_images'. Expect: No matches. rg --type php 'get_template_keys_with_local_images'Length of output: 51
Script:
#!/bin/bash # Description: Perform a comprehensive search for 'get_template_keys_with_local_images' in all PHP files, including different cases. # Test: Expect no matches if the method has been correctly renamed. rg --type php --ignore-case 'get_template_keys_with_local_images'Length of output: 65
Script:
#!/bin/bash # Description: Use ast-grep to find all calls to 'get_template_keys_with_local_images' in PHP files. # Test: Expect no matches if all references have been updated to 'get_template_keys_with_local_png_images'. ast-grep --lang php --pattern '$_->get_template_keys_with_local_images($_, $_)'Length of output: 81
1 participant
This update adds missing thumbnail images for 10 new applications.
Most are now using webp files, so some new logic was added to conditionally use
webpvspng.