Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ export default function MediaEditorCanvas( {
disabled = false,
}: MediaEditorCanvasProps ) {
const { media } = useMediaEditorContext();
const controller = useMediaEditor();
const { aspectRatioValue } = controller.cropOptions;
const cropperImage = controller.state.image;
const { beginGesture, endGesture, setImage } = controller;
const session = useMediaEditor();
const { aspectRatioValue } = session.cropOptions;
const cropperImage = session.cropper.state.image;
const { beginGesture, endGesture, setSourceImage } = session;

// Tracks whether the image pixels have actually loaded. The cropper's
// geometry is driven by the known media dimensions, so its handles and
Expand Down Expand Up @@ -89,12 +89,12 @@ export default function MediaEditorCanvas( {
) {
return;
}
setImage( {
setSourceImage( {
src: mediaUrl,
naturalWidth: mediaWidth,
naturalHeight: mediaHeight,
} );
Comment on lines +92 to 96

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please forgive me if I'm way off with this comment, I may very well have been led astray looking through the code with Claude as my assistant here. But! We call setSourceImage here in this useEffect which now also resets state. But over in cropper.tsx we also set the image when the image loads here:

setImage( {
src,
naturalWidth: size.width,
naturalHeight: size.height,
} );

Previously, my hunch is that setting the image twice was effectively a no-op, whereas now this useEffect could potentially reset state. Is that an issue? I suspect not because we show a loading indicator during load, but just thought I'd mention it in case we want to make sure setSourceImage only fires once 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not way off. I think the second call still is a no-op.

I guess there's a slight risk. It'll reset if the stored width and height don’t match the file’s. So maybe in the case where a user who rotates or picks out a ratio in the fraction of a second before the file's pixels are loaded.

I think the does not clear history when the same image is reported again test covers the general issue.

There's a follow up planned though.

Once the editor supports edits that replace the pixels, e.g., AI, the cropper will have to ignore each "new image" edit since that'd reset everything.

The fix later is for Cropper’s load handler to report the pixel size rather than set the image.

It's on the list over at #79175: "<Cropper> currently treats a new src as a new image."

Does that answer the question? I myself jumped down the rabbit hole!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think that answers the question, thanks! Sounds like something we don't need to worry about for now 🙂

}, [ cropperImage, mediaUrl, mediaWidth, mediaHeight, setImage ] );
}, [ cropperImage, mediaUrl, mediaWidth, mediaHeight, setSourceImage ] );

const isImage = mediaType.type === 'image';

Expand Down Expand Up @@ -159,7 +159,7 @@ export default function MediaEditorCanvas( {
>
<Cropper
src={ mediaUrl }
controller={ controller }
controller={ session.cropper }
aspectRatio={ aspectRatio }
freeformCrop
showGrid="interactive"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function MediaEditorFineRotation( {
onPlacementControlInteraction,
disabled = false,
}: MediaEditorFineRotationProps ) {
const { state, setRotation } = useMediaEditor();
const { state, setRotation } = useMediaEditor().cropper;
// `commitOnKeyUp: false` lets rapid arrow-key adjustments coalesce
// into one undo entry via the gesture idle window. Pointer-up still
// closes pointer drags immediately.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export default function MediaEditorImageControls( {
zoomFactor = DEFAULT_ZOOM_FACTOR,
disabled = false,
}: MediaEditorImageControlsProps ) {
const { state, setFlip, snapRotate90, setZoom } = useMediaEditor();
const { state, setFlip, snapRotate90, setZoom } = useMediaEditor().cropper;
const { aspectRatioValue, setAspectRatioValue, aspectRatioOptions } =
useCropOptions( { aspectRatioPresets } );
const hasAspectRatioControl = ! withLabels && showAspectRatioControl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ function setup(
}

function CurrentState() {
const { state, cropOptions } = useMediaEditor();
const {
cropper: { state },
cropOptions,
} = useMediaEditor();

return (
<>
Expand Down
12 changes: 6 additions & 6 deletions packages/media-editor/src/components/media-editor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ function HistoryActions() {
const { isImage, isUndoRedoDisabled, onReset, isWide, activePanel } =
useMediaEditorFrameContext();
const {
reset,
cropper: { reset },
isDirty,
hasUndo,
hasRedo,
Expand Down Expand Up @@ -435,7 +435,7 @@ function MediaEditorContent( {
noticesPortalElement,
shouldCloseOnEsc = false,
}: MediaEditorProps ) {
const cropper = useMediaEditor();
const session = useMediaEditor();
// Width decides whether the settings panel docks beside the canvas or
// takes the whole body. It docks from `small` (600px): the modal is the
// viewport less a 16px margin either side, so a 320px panel and the
Expand Down Expand Up @@ -516,7 +516,7 @@ function MediaEditorContent( {
[ id ]
);

const hasChanges = cropper.isCropperDirty || hasEdits;
const hasChanges = session.hasOutputEdits || hasEdits;

const { clearEntityRecordEdits, editEntityRecord, invalidateResolution } =
useDispatch( coreStore );
Expand Down Expand Up @@ -602,7 +602,7 @@ function MediaEditorContent( {
resetCropOptions,
} = useCropOptions( { aspectRatioPresets } );
const { isSaving, save: saveMediaEditor } = useSaveMediaEditor( {
cropper,
session,
id,
isImage,
media,
Expand Down Expand Up @@ -656,9 +656,9 @@ function MediaEditorContent( {
return;
}
if ( isRedoShortcut ) {
cropper.redo();
session.redo();
} else {
cropper.undo();
session.undo();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function useCropOptions( {
}: UseCropOptionsArgs = {} ): UseCropOptionsReturn {
const controller = useMediaEditor();
const { aspectRatioValue } = controller.cropOptions;
const cropperImage = controller.state.image;
const cropperImage = controller.cropper.state.image;

const aspectRatioOptions = useMemo(
() => getAspectRatioOptions( aspectRatioPresets ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useCallback, useState } from '@wordpress/element';
import { __, sprintf } from '@wordpress/i18n';
import { store as noticesStore } from '@wordpress/notices';
import type { Media } from '../media-editor-provider';
import type { MediaEditorController } from '../../state';
import type { MediaEditorSession } from '../../state';
import {
buildModifiers,
type Modifier,
Expand Down Expand Up @@ -37,7 +37,7 @@ export interface MediaEditorSaveResult {
}

interface UseSaveMediaEditorArgs {
cropper: MediaEditorController;
session: MediaEditorSession;
id: number;
isImage: boolean;
media?: Media | null;
Expand All @@ -49,13 +49,14 @@ interface UseSaveMediaEditorReturn {
save: () => Promise< void >;
}

function getCropModifiers( cropper: MediaEditorController ): Modifier[] {
if ( ! cropper.isCropperDirty || ! cropper.state.image ) {
function getCropModifiers( session: MediaEditorSession ): Modifier[] {
const { state } = session.cropper;
if ( ! session.hasOutputEdits || ! state.image ) {
return [];
}
return buildModifiers( cropper.state, {
width: cropper.state.image.naturalWidth,
height: cropper.state.image.naturalHeight,
return buildModifiers( state, {
width: state.image.naturalWidth,
height: state.image.naturalHeight,
} );
}

Expand All @@ -80,7 +81,7 @@ function getMetadataEdits(
}

export function useSaveMediaEditor( {
cropper,
session,
id,
isImage,
media,
Expand All @@ -100,7 +101,7 @@ export function useSaveMediaEditor( {
setIsSaving( true );
try {
let saved: Media | null | undefined;
const modifiers = getCropModifiers( cropper );
const modifiers = getCropModifiers( session );
const previous =
modifiers.length > 0 && media
? {
Expand Down Expand Up @@ -155,7 +156,7 @@ export function useSaveMediaEditor( {

if ( next && next.id ) {
if ( next.id === id ) {
cropper.reset();
session.cropper.reset();
}
onSaved?.( {
id: next.id,
Expand Down Expand Up @@ -193,7 +194,6 @@ export function useSaveMediaEditor( {
}, [
clearEntityRecordEdits,
createErrorNotice,
cropper,
id,
isImage,
media,
Expand All @@ -202,6 +202,7 @@ export function useSaveMediaEditor( {
registry,
removeAllNotices,
saveEditedEntityRecord,
session,
] );

return { isSaving, save };
Expand Down
8 changes: 6 additions & 2 deletions packages/media-editor/src/image-editor/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
| React cropper | `src/image-editor/react/` | DOM measurement, interaction hooks, overlays, stencil rendering, and the pure `useCropperReducer` hook. |
| Media editor state | `src/state/` | Composite state for the media editor: cropper state, crop options, undo/redo, and gesture boundaries. |

The cropper layer does not own editor history. History belongs to the composite media editor controller because sidebar controls and cropper gestures need one shared undo stack.
The cropper layer does not own editor history. History belongs to the media editor session because sidebar controls and cropper gestures need one shared undo stack.

## Coordinate Spaces

Expand Down Expand Up @@ -37,6 +37,10 @@ The render path uses focused calculations for CSS transforms, stencil placement,

The core reducer is framework-agnostic and serializable. React and media-editor layers add DOM measurement, gestures, undo/redo, save behavior, and UI state.

### Session

`useMediaEditorState()` returns the media editor session. The session owns the source image, every editing slice, the shared undo/redo history, gesture boundaries, and whether the edits change the saved image (`hasOutputEdits`). The cropper is one participant: `session.cropper` is a geometry-only `CropperController` that a `<Cropper>` takes via its `controller` prop, and its `isDirty` covers geometry alone. Replacing the source image (`setSourceImage`, which the cropper's `setImage` also calls) starts a fresh session with default geometry and crop options, a clean baseline, and no history. Save code asks the session, not the cropper, whether image output needs saving.

### Viewport

Viewport pan/zoom lets the user inspect or follow the crop area without changing the crop output. It is not part of export and should not create undo entries.
Expand All @@ -52,4 +56,4 @@ These APIs let code inside `@wordpress/media-editor` compose the cropper. They a
| Crop data for REST requests | `getSourceRegion()` and `getSourceRegionPercent()`. |
| Canvas export | `exportCroppedImage()` or `applyToCanvas()`. |
| Shared cropper state across internal components | `CropperProvider` / `useCropper()`. |
| Media editor history | `useMediaEditorState()` composite controller. |
| Media editor history | `useMediaEditorState()` session. |
16 changes: 8 additions & 8 deletions packages/media-editor/src/image-editor/docs/recipes.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,29 +76,29 @@ function CropperPanel( { src }: { src: string } ) {
}
```

## Use the Media Editor Composite Controller
## Use the Media Editor Session

Use the media editor controller when cropper state must share history with controls such as aspect ratio.
Use the media editor session when cropper state must share history with controls such as aspect ratio. The session owns history and gesture boundaries; pass its `cropper` controller to `<Cropper>`.

```tsx
import { resolveAspectRatio, useMediaEditor } from '../../state';
import { Cropper } from '../image-editor';

function MediaEditorCanvas( { src }: { src: string } ) {
const controller = useMediaEditor();
const session = useMediaEditor();
const aspectRatio = resolveAspectRatio(
controller.cropOptions.aspectRatioValue,
controller.state.image
session.cropOptions.aspectRatioValue,
session.cropper.state.image
);

return (
<Cropper
src={ src }
controller={ controller }
controller={ session.cropper }
aspectRatio={ aspectRatio }
freeformCrop
onGestureStart={ controller.beginGesture }
onGestureEnd={ controller.endGesture }
onGestureStart={ session.beginGesture }
onGestureEnd={ session.endGesture }
/>
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/media-editor/src/state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ export {
useMediaEditorState,
resolveAspectRatio,
} from './use-media-editor-state';
export type { MediaEditorController } from './use-media-editor-state';
export type { MediaEditorSession } from './use-media-editor-state';
export {
MediaEditorStateProvider,
useMediaEditor,
Expand Down
16 changes: 8 additions & 8 deletions packages/media-editor/src/state/media-editor-state-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { createContext, useContext } from '@wordpress/element';
import type { CropperState } from '../image-editor';
import {
useMediaEditorState,
type MediaEditorController,
type MediaEditorSession,
} from './use-media-editor-state';
import type { CropOptionsSlice } from './types';

const MediaEditorStateContext = createContext< MediaEditorController | null >(
const MediaEditorStateContext = createContext< MediaEditorSession | null >(
null
);

Expand All @@ -20,9 +20,9 @@ interface MediaEditorStateProviderProps {
}

/**
* Provider that vends the composite media-editor controller via
* context. Wrap the media editor in this so every consumer
* (canvas, toolbar, sidebar) reads from the same store.
* Provider that vends the media editor session via context. Wrap
* the media editor in this so every consumer (canvas, toolbar,
* sidebar) reads from the same store.
*
* @param props
* @param props.initialCropperState
Expand All @@ -47,12 +47,12 @@ export function MediaEditorStateProvider( {
}

/**
* Consume the composite media-editor controller. Throws if used
* Consume the media editor session. Throws if used
* outside a `<MediaEditorStateProvider>`.
*
* @return The composite controller.
* @return The media editor session.
*/
export function useMediaEditor(): MediaEditorController {
export function useMediaEditor(): MediaEditorSession {
const context = useContext( MediaEditorStateContext );
if ( ! context ) {
throw new Error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useEffect } from '@wordpress/element';
import { Cropper } from '../../image-editor';
import {
useMediaEditorState,
type MediaEditorController,
type MediaEditorSession,
} from '../use-media-editor-state';
// Browser Mode needs the package's real styles for layout and transitions.
// eslint-disable-next-line @wordpress/no-non-module-stylesheet-imports
Expand All @@ -23,15 +23,15 @@ const IMAGE = {
function CropperWithHistory( {
onController,
}: {
onController: ( controller: MediaEditorController ) => void;
onController: ( controller: MediaEditorSession ) => void;
} ) {
const controller = useMediaEditorState( { cropper: { image: IMAGE } } );
useEffect( () => onController( controller ), [ controller, onController ] );
return (
<div style={ { width: 648, height: 448 } }>
<Cropper
src={ IMAGE.src }
controller={ controller }
controller={ controller.cropper }
freeformCrop
onGestureStart={ controller.beginGesture }
onGestureEnd={ controller.endGesture }
Expand All @@ -42,8 +42,8 @@ function CropperWithHistory( {

describe( 'useMediaEditorState with a Cropper', () => {
it( 'keeps redo steps after redoing a handle crop that turns on pixel snapping', async () => {
let controller!: MediaEditorController;
const onController = ( next: MediaEditorController ) => {
let controller!: MediaEditorSession;
const onController = ( next: MediaEditorSession ) => {
controller = next;
};
await render( <CropperWithHistory onController={ onController } /> );
Expand All @@ -61,11 +61,11 @@ describe( 'useMediaEditorState with a Cropper', () => {
'{/Shift}'
);
await waitFor( () =>
expect( controller.state.zoom ).toBeGreaterThan( 1 )
expect( controller.cropper.state.zoom ).toBeGreaterThan( 1 )
);

act( () =>
controller.setFlip( { horizontal: true, vertical: false } )
controller.cropper.setFlip( { horizontal: true, vertical: false } )
);

for ( let i = 0; i < 10 && controller.hasUndo; i++ ) {
Expand Down
Loading
Loading