fix: add cimo metadata to admin media in grid view #20
Conversation
WalkthroughThis refactoring introduces a reusable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Note Free review on us!CodeRabbit is offering free reviews until Wed Dec 17 2025 to showcase some of the refinements we've made. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/admin/js/media-manager/sidebar-info.js(1 hunks)
🔇 Additional comments (4)
src/admin/js/media-manager/sidebar-info.js (4)
65-88: LGTM - Solid guard checks and fallback logic.The duplicate prevention and fallback to cached metadata appropriately addresses the issue of newly uploaded images not having Cimo data immediately available on the model.
179-196: Verifyview.modelexists in template context.In WordPress Backbone media views, the
templatemethod typically receives serialized model data (fromtoJSON()), not the view object itself. Ifviewis just the model attributes object, thenview.modelon line 190 would beundefined, causinginjectCimoMetadatato exit early without injecting metadata.Compare with the TwoColumn approach (line 212) which correctly accesses
this.modelwithinrender().Please verify this works correctly in the editor media modal. If
view.modelis indeed undefined, consider restructuring to match the TwoColumn pattern:wp.media.view.Attachment.Details = wp.media.view.Attachment.Details.extend( { - template( view ) { - const html = wp.media.template( 'attachment-details' )( view ) - const dom = document.createElement( 'div' ) - dom.innerHTML = html - - const container = dom.querySelector( '.attachment-info' ) - - injectCimoMetadata( { - model: view.model, - container, - } ) - - return dom.innerHTML - }, + render() { + wp.media.view.Attachment.Details.__super__.render.apply( this, arguments ) + + const container = this.el.querySelector( '.attachment-info' ) + + injectCimoMetadata( { + model: this.model, + container, + } ) + + return this + }, } )
169-177: LGTM - Thorough guard check.The expanded guard correctly verifies the entire chain of
wp.media.view.Attachment.Detailsbefore proceeding.
198-219: LGTM - TwoColumn extension follows correct Backbone pattern.Properly stores the original constructor, extends it, calls parent render via prototype, and accesses
this.modelappropriately.
| const optimizationSavings = customMetadata.compressionSavings | ||
| ? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 ) | ||
| : null | ||
|
|
||
| const kbSaved = formatFilesize( | ||
| customMetadata.originalFilesize - customMetadata.convertedFilesize, | ||
| 1, | ||
| true | ||
| ) | ||
|
|
||
| const optimizationSavingsClass = | ||
| optimizationSavings > 0 | ||
| ? 'cimo-optimization-savings-up' | ||
| : 'cimo-optimization-savings-down' |
There was a problem hiding this comment.
Edge cases: handle compressionSavings === 0 and undefined filesizes.
Two potential edge cases:
- If
compressionSavingsis exactly0, it's falsy and results innull, causing "Saved null%" to render. - If
originalFilesizeorconvertedFilesizeis undefined, the subtraction yieldsNaN, causingformatFilesizeto returnNaN undefined.
Consider defensive handling:
-const optimizationSavings = customMetadata.compressionSavings
- ? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 )
- : null
+const optimizationSavings = typeof customMetadata.compressionSavings === 'number'
+ ? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 )
+ : null
-const kbSaved = formatFilesize(
- customMetadata.originalFilesize - customMetadata.convertedFilesize,
+const originalSize = parseInt( customMetadata.originalFilesize ) || 0
+const convertedSize = parseInt( customMetadata.convertedFilesize ) || 0
+const kbSaved = formatFilesize(
+ originalSize - convertedSize,
1,
true
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const optimizationSavings = customMetadata.compressionSavings | |
| ? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 ) | |
| : null | |
| const kbSaved = formatFilesize( | |
| customMetadata.originalFilesize - customMetadata.convertedFilesize, | |
| 1, | |
| true | |
| ) | |
| const optimizationSavingsClass = | |
| optimizationSavings > 0 | |
| ? 'cimo-optimization-savings-up' | |
| : 'cimo-optimization-savings-down' | |
| const optimizationSavings = typeof customMetadata.compressionSavings === 'number' | |
| ? ( 100 - ( customMetadata.compressionSavings * 100 ) ).toFixed( 2 ) | |
| : null | |
| const originalSize = parseInt( customMetadata.originalFilesize ) || 0 | |
| const convertedSize = parseInt( customMetadata.convertedFilesize ) || 0 | |
| const kbSaved = formatFilesize( | |
| originalSize - convertedSize, | |
| 1, | |
| true | |
| ) | |
| const optimizationSavingsClass = | |
| optimizationSavings > 0 | |
| ? 'cimo-optimization-savings-up' | |
| : 'cimo-optimization-savings-down' |
fixes #4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.