-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Media Editor Modal: Add an Advanced Panel with top/left and width/height controls #77778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from 2 commits
7a585da
40acd1f
3888a23
75e346c
d74a860
b471032
1029031
a82212b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { | ||
| __experimentalInputControlSuffixWrapper as InputControlSuffixWrapper, | ||
| __experimentalNumberControl as NumberControl, | ||
| Flex, | ||
| FlexItem, | ||
| PanelBody, | ||
| } from '@wordpress/components'; | ||
| import { useMemo } from '@wordpress/element'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { Stack } from '@wordpress/ui'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { useCropper } from '../../image-editor'; | ||
| import { getCropPixels, pixelsToCropRect } from '../../utils/crop-pixels'; | ||
|
|
||
| interface CropAdvancedPanelProps { | ||
| onPlacementControlInteraction?: () => void; | ||
| } | ||
|
|
||
| const pxSuffix = <InputControlSuffixWrapper>px</InputControlSuffixWrapper>; | ||
|
|
||
| export default function CropAdvancedPanel( { | ||
| onPlacementControlInteraction, | ||
| }: CropAdvancedPanelProps ) { | ||
| const { state, setCropRect } = useCropper(); | ||
|
|
||
| const pixels = useMemo( () => { | ||
| if ( ! state.image ) { | ||
| return null; | ||
| } | ||
| const imageSize = { | ||
| width: state.image.naturalWidth, | ||
| height: state.image.naturalHeight, | ||
| }; | ||
| const raw = getCropPixels( state, imageSize ); | ||
| return { | ||
| x: Math.round( raw.x ), | ||
| y: Math.round( raw.y ), | ||
| width: Math.round( raw.width ), | ||
| height: Math.round( raw.height ), | ||
| snapW: Math.round( raw.snapBBoxWidth ), | ||
| snapH: Math.round( raw.snapBBoxHeight ), | ||
| }; | ||
| }, [ state ] ); | ||
|
|
||
| if ( ! pixels ) { | ||
| return null; | ||
| } | ||
|
|
||
| const { x, y, width, height, snapW, snapH } = pixels; | ||
|
|
||
| const handleChange = | ||
| ( field: 'x' | 'y' | 'width' | 'height' ) => | ||
| ( nextValue: string | undefined ) => { | ||
| const parsed = parseInt( nextValue ?? '', 10 ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was also inputting non finite values like Maybe we could use const next = Number( nextValue );
if ( ! Number.isFinite( next ) ) {
return;
} |
||
| if ( isNaN( parsed ) || ! state.image ) { | ||
| return; | ||
| } | ||
|
|
||
| const imageSize = { | ||
| width: state.image.naturalWidth, | ||
| height: state.image.naturalHeight, | ||
| }; | ||
|
|
||
| const newRect = pixelsToCropRect( | ||
| { | ||
| x: field === 'x' ? parsed : x, | ||
| y: field === 'y' ? parsed : y, | ||
| width: field === 'width' ? parsed : width, | ||
| height: field === 'height' ? parsed : height, | ||
| }, | ||
| state, | ||
| imageSize | ||
| ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if the cropper API needs something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. For now, I was trying to see how far I can get without updating the cropper API... but if this is a common problem for consumers to deal with, then yeah maybe adding it the API might make sense 🤔 |
||
|
|
||
| setCropRect( newRect ); | ||
| onPlacementControlInteraction?.(); | ||
| }; | ||
|
|
||
| return ( | ||
| <PanelBody | ||
| title={ __( 'Advanced' ) } | ||
| initialOpen={ false } | ||
| className="media-editor-crop-advanced-panel" | ||
| > | ||
| <Stack direction="column" gap="sm"> | ||
| <Flex gap={ 2 } align="flex-start"> | ||
| <FlexItem isBlock> | ||
| <NumberControl | ||
| __next40pxDefaultSize | ||
| label={ __( 'Left' ) } | ||
| aria-label={ __( 'Crop left position' ) } | ||
| value={ x } | ||
| min={ 0 } | ||
| max={ Math.max( 0, snapW - width ) } | ||
| step={ 1 } | ||
| onChange={ handleChange( 'x' ) } | ||
| suffix={ pxSuffix } | ||
| /> | ||
| </FlexItem> | ||
| <FlexItem isBlock> | ||
| <NumberControl | ||
| __next40pxDefaultSize | ||
| label={ __( 'Top' ) } | ||
| aria-label={ __( 'Crop top position' ) } | ||
| value={ y } | ||
| min={ 0 } | ||
| max={ Math.max( 0, snapH - height ) } | ||
| step={ 1 } | ||
| onChange={ handleChange( 'y' ) } | ||
| suffix={ pxSuffix } | ||
| /> | ||
| </FlexItem> | ||
| </Flex> | ||
| <Flex gap={ 2 } align="flex-start"> | ||
| <FlexItem isBlock> | ||
| <NumberControl | ||
| __next40pxDefaultSize | ||
| label={ __( 'Width' ) } | ||
| value={ width } | ||
| min={ 1 } | ||
| max={ Math.max( 1, snapW - x ) } | ||
| step={ 1 } | ||
| onChange={ handleChange( 'width' ) } | ||
| suffix={ pxSuffix } | ||
| /> | ||
| </FlexItem> | ||
| <FlexItem isBlock> | ||
| <NumberControl | ||
| __next40pxDefaultSize | ||
| label={ __( 'Height' ) } | ||
| value={ height } | ||
| min={ 1 } | ||
| max={ Math.max( 1, snapH - y ) } | ||
| step={ 1 } | ||
| onChange={ handleChange( 'height' ) } | ||
| suffix={ pxSuffix } | ||
| /> | ||
| </FlexItem> | ||
| </Flex> | ||
| </Stack> | ||
| </PanelBody> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ import { | |
| ORIGINAL_ASPECT_RATIO, | ||
| } from '../../image-editor/core/constants'; | ||
| import type { AspectRatioPreset } from '../../image-editor/core/constants'; | ||
| import CropAdvancedPanel from './crop-advanced-panel'; | ||
|
|
||
| export interface MediaEditorCropPanelProps { | ||
| /** | ||
|
|
@@ -138,6 +139,9 @@ export default function MediaEditorCropPanel( { | |
| checked={ freeformCrop } | ||
| onChange={ onFreeformChange } | ||
| /> | ||
| <CropAdvancedPanel | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not for this PR, but do you think eventually the default crop controls should be in a collapsible component too (default open)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think once we've got this PR in, it'd be good to put our heads together on what other controls are missing, and how the overall area should look and feel 👍 |
||
| onPlacementControlInteraction={ onPlacementControlInteraction } | ||
| /> | ||
| </Stack> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import type { CropperState, NormalizedRect, Size } from '../image-editor'; | ||
| import { getRotatedBBox } from '../image-editor/core/camera'; | ||
|
|
||
| /** | ||
| * The crop rectangle expressed as pixel dimensions in the snap-rotation | ||
| * bounding-box frame. This is the frame the stencil lives in — the same | ||
| * one `buildModifiers` uses to construct the server crop payload. | ||
| * | ||
| * For 0° (and 180°) rotation the snap-AABB equals the source image, so | ||
| * these values are source pixels. For 90°/270° the axes swap. For | ||
| * non-snap rotations (e.g. 45°) the snap-AABB is the nearest 90° AABB, | ||
| * which is how the stencil is positioned. | ||
| */ | ||
| export interface CropPixels { | ||
| /** X offset of the crop in the snap-rotation bounding box, in pixels. */ | ||
| x: number; | ||
| /** Y offset of the crop in the snap-rotation bounding box, in pixels. */ | ||
| y: number; | ||
| /** Width of the crop, in pixels. */ | ||
| width: number; | ||
| /** Height of the crop, in pixels. */ | ||
| height: number; | ||
| /** Width of the snap-rotation bounding box, in pixels. */ | ||
| snapBBoxWidth: number; | ||
| /** Height of the snap-rotation bounding box, in pixels. */ | ||
| snapBBoxHeight: number; | ||
| } | ||
|
|
||
| /** | ||
| * Convert the cropper's normalized cropRect to pixel dimensions in the | ||
| * snap-rotation bounding-box frame. | ||
| * | ||
| * This is the shared math used by `buildModifiers` (to construct the server | ||
| * crop payload) and by the Advanced crop panel (to display and accept | ||
| * pixel-value input). Both callers must stay in sync: if the conversion | ||
| * changes, the server payload and the UI inputs change together. | ||
| * | ||
| * @param state The current cropper state. | ||
| * @param imageSize Natural dimensions of the source image. | ||
| * @return Pixel dimensions of the crop in the snap-rotation frame. | ||
| */ | ||
| export function getCropPixels( | ||
| state: CropperState, | ||
| imageSize: Size | ||
| ): CropPixels { | ||
| const { cropRect, pan, zoom, rotation } = state; | ||
| const snapRotation = Math.round( rotation / 90 ) * 90; | ||
| const { width: snapBBoxWidth, height: snapBBoxHeight } = getRotatedBBox( | ||
| imageSize.width, | ||
| imageSize.height, | ||
| snapRotation | ||
| ); | ||
| const imgLeft = 0.5 + pan.x - zoom / 2; | ||
| const imgTop = 0.5 + pan.y - zoom / 2; | ||
|
|
||
| return { | ||
| x: ( ( cropRect.x - imgLeft ) / zoom ) * snapBBoxWidth, | ||
| y: ( ( cropRect.y - imgTop ) / zoom ) * snapBBoxHeight, | ||
| width: ( cropRect.width / zoom ) * snapBBoxWidth, | ||
| height: ( cropRect.height / zoom ) * snapBBoxHeight, | ||
| snapBBoxWidth, | ||
| snapBBoxHeight, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Convert pixel dimensions in the snap-rotation bounding-box frame back to | ||
| * a normalized `NormalizedRect`. Exact inverse of `getCropPixels`. | ||
| * | ||
| * @param pixels Crop position and size in snap-rotation pixels. | ||
| * @param pixels.x X offset of the crop. | ||
| * @param pixels.y Y offset of the crop. | ||
| * @param pixels.width Width of the crop. | ||
| * @param pixels.height Height of the crop. | ||
| * @param state The current cropper state (provides zoom, pan, rotation). | ||
| * @param imageSize Natural dimensions of the source image. | ||
| * @return The normalized crop rectangle. | ||
| */ | ||
| export function pixelsToCropRect( | ||
| pixels: { x: number; y: number; width: number; height: number }, | ||
| state: CropperState, | ||
| imageSize: Size | ||
| ): NormalizedRect { | ||
| const { pan, zoom, rotation } = state; | ||
| const snapRotation = Math.round( rotation / 90 ) * 90; | ||
| const { width: snapBBoxWidth, height: snapBBoxHeight } = getRotatedBBox( | ||
| imageSize.width, | ||
| imageSize.height, | ||
| snapRotation | ||
| ); | ||
| const imgLeft = 0.5 + pan.x - zoom / 2; | ||
| const imgTop = 0.5 + pan.y - zoom / 2; | ||
|
|
||
| return { | ||
| x: ( pixels.x / snapBBoxWidth ) * zoom + imgLeft, | ||
| y: ( pixels.y / snapBBoxHeight ) * zoom + imgTop, | ||
| width: ( pixels.width / snapBBoxWidth ) * zoom, | ||
| height: ( pixels.height / snapBBoxHeight ) * zoom, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dumb question: are we talking only about the cropping box for these values? E.g., the width of the rectangle (rather than the width of the underlying cropped area as a fraction of the original image width)
What should happen when freeform crop is off? Or when an aspect ratio is selected?
For the former and latter, I guess width/height don't make much sense. Should they be disabled? (?)
I guess I'm not sure what should happen - right now I can enter a value manually when freeform is off
Kapture.2026-04-29.at.14.57.29.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, maybe the input controls need to be clamped to the aspect ratio in that case. So if I update width, height automatically updates too, or if I update height, then width updates... I'll hack around.