From 2321be4f93c090bfce95407f6ddf7f4a3154a1ae Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 16 Aug 2024 15:07:11 +0200 Subject: [PATCH] Use original URL and ID when compressing image (#620) --- inc/functions.php | 31 +++++++ .../src/block-media-panel/add-poster.tsx | 17 +++- .../animated-gif-converter.tsx | 17 ++-- .../src/block-media-panel/debug-info.tsx | 13 +-- .../block-media-panel/generate-subtitles.tsx | 10 ++- .../src/block-media-panel/mute-video.tsx | 20 +++-- .../block-media-panel/upload-indicator.tsx | 14 +++- .../src/components/approval-dialog/index.tsx | 13 +-- packages/editor/src/utils/hooks.ts | 83 +++++++++++-------- packages/media-utils/src/utils/types.ts | 1 + .../upload-media/src/store/private-actions.ts | 16 +--- 11 files changed, 155 insertions(+), 80 deletions(-) diff --git a/inc/functions.php b/inc/functions.php index d6b25962..827e7e5e 100644 --- a/inc/functions.php +++ b/inc/functions.php @@ -473,6 +473,19 @@ function register_rest_fields(): void { 'get_callback' => __NAMESPACE__ . '\rest_get_attachment_has_transparency', ] ); + + register_rest_field( + 'attachment', + 'mexp_original_url', + [ + 'schema' => [ + 'description' => __( 'URL of the original file if this is an optimized one', 'media-experiments' ), + 'type' => 'boolean', + 'context' => [ 'view', 'edit' ], + ], + 'get_callback' => __NAMESPACE__ . '\rest_get_attachment_original_file', + ] + ); } /** @@ -576,6 +589,24 @@ function rest_get_attachment_has_transparency( array $post ): ?bool { return isset( $meta['has_transparency'] ) ? (bool) $meta['has_transparency'] : null; } +/** + * Returns the URL of the original file if this is an optimized one + * + * @param array $post Post data. + * @return string|null Original URL if applicable. + */ +function rest_get_attachment_original_file( array $post ): ?string { + $original_id = get_post_meta( $post['id'], 'mexp_original_id', true ); + + if ( empty( $original_id ) ) { + return null; + } + + $original_url = wp_get_attachment_url( $original_id ); + + return $original_url ? $original_url : null; +} + /** * Registers additional post meta for the attachment post type. * diff --git a/packages/editor/src/block-media-panel/add-poster.tsx b/packages/editor/src/block-media-panel/add-poster.tsx index 6c8d731a..4605a93d 100644 --- a/packages/editor/src/block-media-panel/add-poster.tsx +++ b/packages/editor/src/block-media-panel/add-poster.tsx @@ -2,6 +2,7 @@ * External dependencies */ import { store as uploadStore } from '@mexp/upload-media'; +import type { RestAttachment } from '@mexp/media-utils'; /** * WordPress dependencies @@ -15,12 +16,13 @@ import { useEffect, useState } from '@wordpress/element'; import { isBlobURL } from '@wordpress/blob'; import { __ } from '@wordpress/i18n'; import { useDispatch } from '@wordpress/data'; +import { useEntityRecord } from '@wordpress/core-data'; import apiFetch from '@wordpress/api-fetch'; /** * Internal dependencies */ -import { useAttachment, useIsUploadingByUrl } from '../utils/hooks'; +import { useIsUploadingByUrl } from '../utils/hooks'; interface AddPosterProps { attributes: { @@ -36,8 +38,17 @@ export function AddPoster( { attributes, setAttributes }: AddPosterProps ) { const [ posterId, setPosterId ] = useState< number | undefined >(); - const attachment = useAttachment( attributes.id ); - const poster = useAttachment( posterId ); + const { record: attachment } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + attributes.id || 0 + ); + + const { record: poster } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + posterId || 0 + ); const { addPosterForExistingVideo } = useDispatch( uploadStore ); diff --git a/packages/editor/src/block-media-panel/animated-gif-converter.tsx b/packages/editor/src/block-media-panel/animated-gif-converter.tsx index 0e61ec96..8bf8e4f0 100644 --- a/packages/editor/src/block-media-panel/animated-gif-converter.tsx +++ b/packages/editor/src/block-media-panel/animated-gif-converter.tsx @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import type { RestAttachment } from '@mexp/media-utils'; + /** * WordPress dependencies */ @@ -5,11 +10,7 @@ import { useEffect } from '@wordpress/element'; import { store as blockEditorStore } from '@wordpress/block-editor'; import { createBlock } from '@wordpress/blocks'; import { useDispatch } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import { useAttachment } from '../utils/hooks'; +import { useEntityRecord } from '@wordpress/core-data'; interface AnimatedGifDetectorProps { id: number; @@ -25,7 +26,11 @@ export function AnimatedGifConverter( { caption, clientId, }: AnimatedGifDetectorProps ) { - const attachment = useAttachment( id ); + const { record: attachment } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + id + ); const isVideo = attachment?.mime_type.startsWith( 'video/' ); diff --git a/packages/editor/src/block-media-panel/debug-info.tsx b/packages/editor/src/block-media-panel/debug-info.tsx index 64e9ea5a..43a7d7cb 100644 --- a/packages/editor/src/block-media-panel/debug-info.tsx +++ b/packages/editor/src/block-media-panel/debug-info.tsx @@ -3,6 +3,7 @@ */ import { Blurhash } from 'react-blurhash'; import type { PropsWithChildren } from 'react'; +import type { RestAttachment } from '@mexp/media-utils'; /** * WordPress dependencies @@ -18,11 +19,7 @@ import { } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { Component, createInterpolateElement } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import { useAttachment } from '../utils/hooks'; +import { useEntityRecord } from '@wordpress/core-data'; interface DebugInfoProps { id: number; @@ -70,7 +67,11 @@ class HideOnError extends Component< PropsWithChildren< {} > > { export function DebugInfo( { id }: DebugInfoProps ) { const { baseControlProps, controlProps } = useBaseControlProps( {} ); - const attachment = useAttachment( id ); + const { record: attachment } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + id + ); if ( ! attachment ) { return null; diff --git a/packages/editor/src/block-media-panel/generate-subtitles.tsx b/packages/editor/src/block-media-panel/generate-subtitles.tsx index a97480f0..6d7b2587 100644 --- a/packages/editor/src/block-media-panel/generate-subtitles.tsx +++ b/packages/editor/src/block-media-panel/generate-subtitles.tsx @@ -1,6 +1,7 @@ /** * External dependencies */ +import type { RestAttachment } from '@mexp/media-utils'; import { store as uploadStore } from '@mexp/upload-media'; /** @@ -9,6 +10,7 @@ import { store as uploadStore } from '@mexp/upload-media'; import type { BlockEditProps } from '@wordpress/blocks'; import { isBlobURL } from '@wordpress/blob'; import { useDispatch } from '@wordpress/data'; +import { useEntityRecord } from '@wordpress/core-data'; import { __, _x } from '@wordpress/i18n'; import { BaseControl, @@ -20,7 +22,7 @@ import { useLayoutEffect } from '@wordpress/element'; /** * Internal dependencies */ -import { useAttachment, useIsUploadingByUrl } from '../utils/hooks'; +import { useIsUploadingByUrl } from '../utils/hooks'; import type { VideoBlock } from '../types'; type GenerateSubtitlesProps = VideoBlock & @@ -32,7 +34,11 @@ export function GenerateSubtitles( { }: GenerateSubtitlesProps ) { const { baseControlProps, controlProps } = useBaseControlProps( {} ); - const post = useAttachment( attributes.id ); + const { record: post } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + attributes.id + ); const url = attributes.src; const isUploading = useIsUploadingByUrl( url ) || isBlobURL( url ); diff --git a/packages/editor/src/block-media-panel/mute-video.tsx b/packages/editor/src/block-media-panel/mute-video.tsx index 0b57700e..d74fa121 100644 --- a/packages/editor/src/block-media-panel/mute-video.tsx +++ b/packages/editor/src/block-media-panel/mute-video.tsx @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { Attachment } from '@mexp/media-utils'; +import type { Attachment, RestAttachment } from '@mexp/media-utils'; import { store as uploadStore } from '@mexp/upload-media'; /** @@ -15,11 +15,12 @@ import { useBaseControlProps, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; +import { useEntityRecord } from '@wordpress/core-data'; /** * Internal dependencies */ -import { useAttachment, useIsUploadingByUrl } from '../utils/hooks'; +import { useIsUploadingByUrl } from '../utils/hooks'; interface MuteVideoProps { id: number; @@ -31,7 +32,11 @@ interface MuteVideoProps { export function MuteVideo( { id, url, poster, onChange }: MuteVideoProps ) { const { baseControlProps, controlProps } = useBaseControlProps( {} ); - const post = useAttachment( id ); + const { record: post } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + id + ); const isUploading = useIsUploadingByUrl( url ) || isBlobURL( url ); @@ -51,12 +56,13 @@ export function MuteVideo( { id, url, poster, onChange }: MuteVideoProps ) { onChange: ( [ media ] ) => onChange( media ), onSuccess: ( [ media ] ) => onChange( media ), additionalData: { - mexp_blurhash: post?.mexp_blurhash, - mexp_dominant_color: post?.mexp_dominant_color, - featured_media: post?.meta.mexp_generated_poster_id, + mexp_blurhash: post.mexp_blurhash, + mexp_dominant_color: post.mexp_dominant_color, + featured_media: post.meta.mexp_generated_poster_id, meta: { + mexp_original_id: post.id, mexp_generated_poster_id: - post?.meta.mexp_generated_poster_id, + post.meta.mexp_generated_poster_id, }, }, } ); diff --git a/packages/editor/src/block-media-panel/upload-indicator.tsx b/packages/editor/src/block-media-panel/upload-indicator.tsx index f17d1131..a9bedaa2 100644 --- a/packages/editor/src/block-media-panel/upload-indicator.tsx +++ b/packages/editor/src/block-media-panel/upload-indicator.tsx @@ -1,14 +1,20 @@ +/** + * External dependencies + */ +import { store as uploadStore } from '@mexp/upload-media'; + /** * WordPress dependencies */ import { isBlobURL } from '@wordpress/blob'; import { Notice } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { useIsUploadingById, useIsUploadingByUrl } from '../utils/hooks'; +import { useIsUploadingByUrl } from '../utils/hooks'; interface UploadIndicatorProps { id: number; @@ -17,7 +23,11 @@ interface UploadIndicatorProps { } export function UploadIndicator( { id, url, poster }: UploadIndicatorProps ) { - const isUploadingById = useIsUploadingById( id ); + const isUploadingById = useSelect( + ( select ) => + id ? select( uploadStore ).isUploadingById( id ) : false, + [ id ] + ); const isUploadingByUrl = useIsUploadingByUrl( url ); const isPosterUploadingByUrl = useIsUploadingByUrl( poster ); const isUploading = isUploadingById || isUploadingByUrl; diff --git a/packages/editor/src/components/approval-dialog/index.tsx b/packages/editor/src/components/approval-dialog/index.tsx index 560a68be..206ead13 100644 --- a/packages/editor/src/components/approval-dialog/index.tsx +++ b/packages/editor/src/components/approval-dialog/index.tsx @@ -5,6 +5,8 @@ import { ReactCompareSlider, ReactCompareSliderImage, } from 'react-compare-slider'; +import type { RestAttachment } from '@mexp/media-utils'; +import { store as uploadStore } from '@mexp/upload-media'; /** * WordPress dependencies @@ -13,14 +15,11 @@ import { Button, Modal } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { createInterpolateElement, useState } from '@wordpress/element'; import { useDispatch, useSelect } from '@wordpress/data'; - -import { store as uploadStore } from '@mexp/upload-media'; +import { useEntityRecord } from '@wordpress/core-data'; /** * Internal dependencies */ -import { useAttachment } from '../../utils/hooks'; - import './editor.css'; const numberFormatter = Intl.NumberFormat( 'en', { @@ -44,7 +43,11 @@ interface ApprovalDialogProps { } export function ApprovalDialog( { id }: ApprovalDialogProps ) { - const post = useAttachment( id ); + const { record: post } = useEntityRecord< RestAttachment | null >( + 'postType', + 'attachment', + id + ); const { isPendingApproval, comparison } = useSelect( ( select ) => ( { // This allows showing only one approval modal at a time if there diff --git a/packages/editor/src/utils/hooks.ts b/packages/editor/src/utils/hooks.ts index 1919b3ed..2d8ed84f 100644 --- a/packages/editor/src/utils/hooks.ts +++ b/packages/editor/src/utils/hooks.ts @@ -11,7 +11,6 @@ import { type Settings, store as coreStore, useEntityProp, - useEntityRecord, useEntityRecords, } from '@wordpress/core-data'; import { useDispatch, useSelect } from '@wordpress/data'; @@ -24,19 +23,6 @@ import { store as blockEditorStore } from '@wordpress/block-editor'; */ import type { BulkOptimizationAttachmentData } from '../types'; -export function useAttachment( id?: number ) { - const { record } = useEntityRecord( 'postType', 'attachment', id || 0 ); - return record as RestAttachment | null; -} - -export function useIsUploadingById( id?: number ) { - return useSelect( - ( select ) => - id ? select( uploadStore ).isUploadingById( id ) : false, - [ id ] - ); -} - export function useIsUploadingByUrl( url?: string ) { return useSelect( ( select ) => { @@ -54,6 +40,17 @@ export function useIsUploadingByUrl( url?: string ) { const EMPTY_ARRAY: never[] = []; +/** + * For a list of attachment objects, enriches them with server-side data. + * + * Since the number of items in the list can change, this uses + * `__experimentalGetEntityRecordNoResolver()` in addition to `useEntityRecords` + * to avoid unnecessary HTTP requests when all the individual items have been + * previously fetched already. + * + * @param attachments List of attachments. + * @param enabled Whether to actually send requests. + */ function useAttachmentsWithEntityRecords( attachments: Partial< BulkOptimizationAttachmentData >[], enabled = true @@ -100,33 +97,47 @@ function useAttachmentsWithEntityRecords( return EMPTY_ARRAY; } - return attachments.map( ( attachment ) => { - const media = - records?.find( ( record ) => record.id === attachment.id ) || - cachedRecords.find( ( record ) => record.id === attachment.id ); + // Add server-side data but remove the ones not found on the server anymore. + return attachments + .map( ( attachment ) => { + const media = + records?.find( ( record ) => record.id === attachment.id ) || + cachedRecords.find( ( record ) => record.id === attachment.id ); - if ( media ) { - // Always use the full URL, in case the block uses a sub-size. - attachment.url = media.source_url; + if ( media ) { + // Always use the full URL, in case the block uses a sub-size. + attachment.url = media.source_url; - if ( media.mexp_filesize ) { - attachment.filesize = media.mexp_filesize; - } + // Always use the original ID and URL in case this image is already an optimized version. + if ( media.mexp_original_url ) { + attachment.url = media.mexp_original_url; + } - if ( media.mexp_filename ) { - attachment.filename = media.mexp_filename; - } + if ( media.mexp_filesize ) { + attachment.filesize = media.mexp_filesize; + } - attachment.additionalData = { - mexp_blurhash: media.mexp_blurhash || undefined, - mexp_dominant_color: media.mexp_dominant_color || undefined, - featured_media: - media.meta.mexp_generated_poster_id || undefined, - }; - } + if ( media.mexp_filename ) { + attachment.filename = media.mexp_filename; + } - return attachment as BulkOptimizationAttachmentData; - } ); + attachment.additionalData = { + meta: { + mexp_original_id: + media.meta.mexp_original_id || attachment.id, + mexp_blurhash: media.mexp_blurhash || undefined, + mexp_dominant_color: + media.mexp_dominant_color || undefined, + featured_media: + media.meta.mexp_generated_poster_id || undefined, + }, + }; + + return attachment as BulkOptimizationAttachmentData; + } + return undefined; + } ) + .filter( ( attachment ) => attachment !== undefined ); } export function useBlockAttachments( clientId?: string ) { diff --git a/packages/media-utils/src/utils/types.ts b/packages/media-utils/src/utils/types.ts index 0dfb8b38..df7d3b38 100644 --- a/packages/media-utils/src/utils/types.ts +++ b/packages/media-utils/src/utils/types.ts @@ -191,6 +191,7 @@ export interface RestAttachment extends WP_REST_API_Attachment { mexp_dominant_color?: string; mexp_is_muted?: boolean; mexp_has_transparency?: boolean; + mexp_original_url: string | null; } type BetterOmit< T, K extends PropertyKey > = { diff --git a/packages/upload-media/src/store/private-actions.ts b/packages/upload-media/src/store/private-actions.ts index af844e5c..09e1eadd 100644 --- a/packages/upload-media/src/store/private-actions.ts +++ b/packages/upload-media/src/store/private-actions.ts @@ -1757,16 +1757,6 @@ export function uploadItem( id: QueueItemId ) { const startTime = performance.now(); - const { poster } = item; - - const additionalData: Record< string, unknown > = { - ...item.additionalData, - meta: { - ...( item.additionalData.meta || {} ), - mexp_original_id: item.sourceAttachmentId || undefined, - }, - }; - const timing: MeasureOptions = { measureName: `Upload item ${ item.file.name }`, startTime, @@ -1782,7 +1772,7 @@ export function uploadItem( id: QueueItemId ) { select.getSettings().mediaUpload( { filesList: [ item.file ], - additionalData, + additionalData: item.additionalData, signal: item.abortController?.signal, onFileChange: ( [ attachment ] ) => { // TODO: Get the poster URL from the ID if one exists already. @@ -1797,8 +1787,8 @@ export function uploadItem( id: QueueItemId ) { */ if ( item.attachment?.poster ) { attachment.poster = item.attachment.poster; - } else if ( poster ) { - attachment.poster = createBlobURL( poster ); + } else if ( item.poster ) { + attachment.poster = createBlobURL( item.poster ); dispatch< CacheBlobUrlAction >( { type: Type.CacheBlobUrl,