Skip to content

feat: add video and audio bulk optimization#46

Open
Arukuen wants to merge 1 commit intodevelopfrom
feat/37-video-audio-bulk
Open

feat: add video and audio bulk optimization#46
Arukuen wants to merge 1 commit intodevelopfrom
feat/37-video-audio-bulk

Conversation

@Arukuen
Copy link
Copy Markdown
Contributor

@Arukuen Arukuen commented Apr 21, 2026

fixes #37

Summary by CodeRabbit

  • Improvements
    • Bulk-optimization list items now display appropriate terminology based on file type: "thumbnail(s) processed" for images and "file(s) processed" for other media files.
    • Error messaging is now more consistent when converted files exceed input file size.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

Updated UI text labels and error messages to reflect broader media type support beyond images. Conditional logic now distinguishes between image media (displaying "thumbnail(s) processed") and non-image media (displaying "file(s) processed"). Renamed error reason constant for consistency.

Changes

Cohort / File(s) Summary
Bulk Optimization UI Labels
src/admin/class-meta-box.php, src/admin/js/media-manager/sidebar-info.js
Added conditional logic to detect image media types (MIME type starting with image/) and display "thumbnail(s) processed" for images or "file(s) processed" for other media types in the bulk-optimization list.
Error Message Naming
src/shared/converters/image-converter.js
Renamed conversion-skip reason constant from 'resulting-image-bigger-than-input' to 'resulting-media-bigger-than-input' to reflect broader media type support.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 Files now speak for videos and sounds,
Where once just images had their grounds,
Thumbnails for pics, files for the rest,
A naming update that feels so blessed! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to add video and audio bulk optimization, but the changes only update display text for thumbnails vs files—no actual video/audio support implementation is present. Either implement the full video/audio bulk optimization feature or retitle to reflect the actual changes (e.g., 'refactor: update bulk optimization labels based on media type').
Linked Issues check ⚠️ Warning The changes do not address the core requirements from #37: no video/audio file type handling, no conversion logic changes, no browser-support filtering, and no evidence of video/audio optimization implementation. Implement the missing functionality: add video/audio file detection, update converters to handle video/audio formats, filter unsupported types, and apply Cimo's video/audio settings during optimization.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive The changes are minimal label updates that address display logic but fall short of the full scope requirement to extend bulk optimization to video and audio files with complete conversion support. Clarify whether this PR is an incomplete first step toward full video/audio support or if the objective scope has changed; align PR description with actual implementation.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/37-video-audio-bulk

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🤖 Pull request artifacts

file commit
pr46-cimo-46-merge.zip 4df7205

github-actions Bot added a commit that referenced this pull request Apr 21, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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 (1)
src/admin/class-meta-box.php (1)

209-215: Translator string extraction & image-only emoji.

Two minor concerns:

  1. Embedding esc_html__() calls inside a ternary as the sprintf format argument works at runtime but can confuse some gettext scanners/translator context. Safer pattern:
♻️ Proposed refactor
-echo '🏞️ ' . sprintf(
-    /* translators: %s: bulk optimization count */
-    $is_image_media
-        ? esc_html__( '%s thumbnail(s) processed', 'cimo-image-optimizer' )
-        : esc_html__( '%s file(s) processed', 'cimo-image-optimizer' ),
-    '<span class="cimo-value">' . esc_html( $bulk_optimization_count ) . '</span>'
-);
+/* translators: %s: bulk optimization count */
+$bulk_label = $is_image_media
+    ? esc_html__( '%s thumbnail(s) processed', 'cimo-image-optimizer' )
+    : esc_html__( '%s file(s) processed', 'cimo-image-optimizer' );
+$icon = $is_image_media ? '🏞️' : '📦';
+echo $icon . ' ' . sprintf(
+    $bulk_label,
+    '<span class="cimo-value">' . esc_html( $bulk_optimization_count ) . '</span>'
+);
  1. The 🏞️ emoji is image-themed; for video/audio it reads oddly alongside "file(s) processed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/admin/class-meta-box.php` around lines 209 - 215, Extract the
translatable format string out of the ternary and store it in a variable (e.g.,
$format) by assigning $format = $is_image_media ? esc_html__( '%s thumbnail(s)
processed', 'cimo-image-optimizer' ) : esc_html__( '%s file(s) processed',
'cimo-image-optimizer' ); then call echo with sprintf( $format, '<span
class="cimo-value">' . esc_html( $bulk_optimization_count ) . '</span>' ); also
avoid the image-specific 🏞️ emoji for non-image media—either use a neutral icon
(e.g., 📁) or conditionally prepend the emoji based on $is_image_media so
video/audio lines aren’t mislabeled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/admin/js/media-manager/sidebar-info.js`:
- Line 189: Replace the hardcoded 🏞️ emoji and literal strings with a neutral
or media-aware icon and proper i18n calls: use the existing isImageMedia boolean
to choose an emoji (e.g., '🏞️' for images, '🎞️' for video, '🎵' for audio or a
neutral '📦' when unknown) and wrap the label text in the translation helper
(use the same JS i18n function pattern used elsewhere) instead of the raw
`'thumbnail(s) processed'` / `'file(s) processed'`; update the template
expression that references customMetadata.bulk_optimization and escape(...) so
it outputs the chosen icon and translated label while preserving the existing
escape(...) usage and semantics.

---

Nitpick comments:
In `@src/admin/class-meta-box.php`:
- Around line 209-215: Extract the translatable format string out of the ternary
and store it in a variable (e.g., $format) by assigning $format =
$is_image_media ? esc_html__( '%s thumbnail(s) processed',
'cimo-image-optimizer' ) : esc_html__( '%s file(s) processed',
'cimo-image-optimizer' ); then call echo with sprintf( $format, '<span
class="cimo-value">' . esc_html( $bulk_optimization_count ) . '</span>' ); also
avoid the image-specific 🏞️ emoji for non-image media—either use a neutral icon
(e.g., 📁) or conditionally prepend the emoji based on $is_image_media so
video/audio lines aren’t mislabeled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 856ef830-aa05-4e4c-af56-dcba53d8bee2

📥 Commits

Reviewing files that changed from the base of the PR and between 9cea982 and 4df7205.

📒 Files selected for processing (3)
  • src/admin/class-meta-box.php
  • src/admin/js/media-manager/sidebar-info.js
  • src/shared/converters/image-converter.js

html += `
<li class="cimo-bulk-optimization-number">
🏞️ <span class="cimo-value">${ escape( Object.keys( customMetadata.bulk_optimization ).length.toString() ) }</span> thumbnail(s) processed
🏞️ <span class="cimo-value">${ escape( Object.keys( customMetadata.bulk_optimization ).length.toString() ) }</span> ${ escape( isImageMedia ? 'thumbnail(s) processed' : 'file(s) processed' ) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Emoji still image-specific for non-image media.

The 🏞️ (landscape) emoji prefix remains for video/audio too, which visually contradicts the new "file(s) processed" label. Consider a neutral icon (e.g., 📦 or 🎞️/🎵 by media type) when !isImageMedia.

Also note: the literal strings 'thumbnail(s) processed' and 'file(s) processed' bypass __() i18n (unlike the PHP counterpart which uses esc_html__). Pre-existing pattern nearby, but worth aligning for translators.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/admin/js/media-manager/sidebar-info.js` at line 189, Replace the
hardcoded 🏞️ emoji and literal strings with a neutral or media-aware icon and
proper i18n calls: use the existing isImageMedia boolean to choose an emoji
(e.g., '🏞️' for images, '🎞️' for video, '🎵' for audio or a neutral '📦' when
unknown) and wrap the label text in the translation helper (use the same JS i18n
function pattern used elsewhere) instead of the raw `'thumbnail(s) processed'` /
`'file(s) processed'`; update the template expression that references
customMetadata.bulk_optimization and escape(...) so it outputs the chosen icon
and translated label while preserving the existing escape(...) usage and
semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add video and audio support in Bulk Optimization

1 participant