Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -786,7 +786,7 @@ export class InteractionController {
...currentState,
pan: {
x: currentState.pan.x,
y: currentState.pan.y - this.keyboardStep,
y: currentState.pan.y + this.keyboardStep,
},
},
getImageSizeFromState( currentState ),
Expand All @@ -805,7 +805,7 @@ export class InteractionController {
...currentState,
pan: {
x: currentState.pan.x,
y: currentState.pan.y + this.keyboardStep,
y: currentState.pan.y - this.keyboardStep,
},
},
getImageSizeFromState( currentState ),
Expand All @@ -823,7 +823,7 @@ export class InteractionController {
{
...currentState,
pan: {
x: currentState.pan.x - this.keyboardStep,
x: currentState.pan.x + this.keyboardStep,
y: currentState.pan.y,
},
},
Expand All @@ -842,7 +842,7 @@ export class InteractionController {
{
...currentState,
pan: {
x: currentState.pan.x + this.keyboardStep,
x: currentState.pan.x - this.keyboardStep,
y: currentState.pan.y,
},
},
Expand Down Expand Up @@ -883,6 +883,9 @@ export class InteractionController {
}
case 'r':
case 'R': {
if ( e.metaKey || e.ctrlKey || e.altKey || e.shiftKey ) {
break;
}
e.preventDefault();
this.options.dispatch( {
type: 'SNAP_ROTATE_90',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,10 +520,10 @@ describe( 'InteractionController', () => {
const call = dispatchMock.mock.calls.find(
( c ) => c[ 0 ].type === 'SET_PAN'
);
// ArrowUp decreases y by keyboardStep (0.05 default).
// ArrowUp scrolls the viewport up — image moves down, so y increases.
expect(
( call![ 0 ].payload as { y: number } ).y
).toBeLessThanOrEqual( 0 );
).toBeGreaterThanOrEqual( 0 );
} );

it( 'dispatches SET_CROP on ArrowDown', () => {
Expand All @@ -550,10 +550,10 @@ describe( 'InteractionController', () => {
const call = dispatchMock.mock.calls.find(
( c ) => c[ 0 ].type === 'SET_PAN'
);
// ArrowLeft decreases x by keyboardStep.
// ArrowLeft scrolls the viewport left — image moves right, so x increases.
expect(
( call![ 0 ].payload as { x: number } ).x
).toBeLessThanOrEqual( 0 );
).toBeGreaterThanOrEqual( 0 );
} );

it( 'dispatches SET_CROP on ArrowRight', () => {
Expand Down Expand Up @@ -621,6 +621,20 @@ describe( 'InteractionController', () => {
} );
} );

it.each( [ 'metaKey', 'ctrlKey', 'altKey', 'shiftKey' ] )(
'does not rotate when %s is held with r',
( modifier ) => {
const state = makeState( { rotation: 0 } );
const { controller } = createController( state );

controller.handleKeyDown(
createKeyboardEvent( 'r', { [ modifier ]: true } )
);

expect( dispatchMock ).not.toHaveBeenCalled();
}
);

it( 'respects custom keyboardStep (read lazily)', () => {
const state = makeState( { zoom: 2 } );
const { controller, opts } = createController( state, {
Expand All @@ -635,10 +649,10 @@ describe( 'InteractionController', () => {
const call = dispatchMock.mock.calls.find(
( c ) => c[ 0 ].type === 'SET_PAN'
);
// At zoom=2 with full crop rect, maxX = 0.25.
// 0 + 0.1 = 0.1, within bounds.
// ArrowRight scrolls the viewport right — image moves left, so x decreases.
// 0 - 0.1 = -0.1, within bounds.
expect( ( call![ 0 ].payload as { x: number } ).x ).toBeCloseTo(
0.1
-0.1
);
} );

Expand Down
2 changes: 2 additions & 0 deletions packages/media-editor/src/image-editor/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,6 @@ export interface StencilProps {
maxX: number;
maxY: number;
};
/** Called when Escape is pressed on a resize handle. */
onEscape?: () => void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ $handle-touch-target-size: 44px;
&__handle {
// Reset native <button> UA styles — the handle is a button for
// accessibility (native keyboard focus, button semantics), but
// it's styled as a small draggable square.
// it's styled as a small draggable circle.
appearance: none;
margin: 0;
padding: 0;
Expand All @@ -99,8 +99,22 @@ $handle-touch-target-size: 44px;
height: 12px;
background: rgba(255, 255, 255, 0.6);
border: 1px solid rgba(255, 255, 255, 0.9);
border-radius: 50%;
box-sizing: border-box;
pointer-events: auto;
transition: background-color 0.15s ease, box-shadow 0.15s ease;

&:focus {
outline: none;
}

// Double ring (white inner + theme-color outer) for legibility
// over the dark dimming overlay.
&:focus-visible {
background: var(--wp-admin-theme-color, #007cba);
border-color: transparent;
box-shadow: 0 0 0 2px #fff, 0 0 0 4px var(--wp-admin-theme-color, #007cba);
}
Comment on lines +105 to +117

@ramonjd ramonjd Apr 24, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you think this should be default, or overridden by the consumer?

My thinking was we'd have all interactive elements as vanilla as they can be so different use cases (e.g., mobile might want different handles) can overwrite the CSS with minimal changes.

No blockers here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, good point! Yes I was a bit focused on hacking it in there I completely forgot about our layers of abstraction here and that image-editor might one day be an independent package. I'll have a play around, but yes, I reckon we want a default styling for the component itself, and the WP implementation can override with the wp-admin color 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went a little back-and-forth playing around with this one and settled on using a CSS variable named --wp-image-editor-focus-color and falling back to --wp-admin-theme-color if available (pushed in 5fe6fe4). I'm possibly overthinking it, but here's what was on my mind:

  • If/when we move image-editor to be used as a package, it could be that plugins might make use of the image editor, and it'd be good for it to "just work" with a WordPress context without any extra configuration
  • But we also want the image editor to be easily configurable outside of WordPress and to not depend on WP existing

So that's where I landed. I imagine we'll be tweaking the API of this a lot in coming PRs but it seemed an okay balance to me for now.

I'll aim to merge this once tests pass, but let me know if you feel otherwise!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!


// Extend touch target to the minimum accessible hit area.
&::before {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,14 @@ function CropperInner(
};
}, [] );

/**
* Handle Escape on a resize handle — return focus to the canvas so
* arrow keys pan the image rather than resize.
*/
const handleEscape = useCallback( () => {
canvasRef.current?.focus( { preventScroll: true } );
}, [] );

/**
* Handle resize end — settle the crop rect (re-center, fill height).
*/
Expand Down Expand Up @@ -465,6 +473,7 @@ function CropperInner(
onCropChange={ handleCropChange }
onResizeStart={ onGestureStart }
onResizeEnd={ handleResizeEnd }
onEscape={ handleEscape }
aspectRatio={ aspectRatio }
freeformCrop={ freeformCrop }
stencilTransition={ settleStencilTransition }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,23 @@ import {

/**
* Corner handle positions only — used when aspect ratio is locked.
* Ordered clockwise from top-left for logical tab order.
*/
const CORNER_POSITIONS: HandlePosition[] = [ 'nw', 'ne', 'sw', 'se' ];
const CORNER_POSITIONS: HandlePosition[] = [ 'nw', 'ne', 'se', 'sw' ];

/**
* All handle positions rendered by the stencil.
* Ordered clockwise from top-left for logical tab order.
*/
const ALL_POSITIONS: HandlePosition[] = [
'n',
's',
'e',
'w',
'nw',
'n',
'ne',
'sw',
'e',
'se',
's',
'sw',
'w',
];

/**
Expand Down Expand Up @@ -62,10 +64,11 @@ function getHandleLabel( pos: HandlePosition ): string {
}
}

/**
* Step size for keyboard-driven handle resize, in normalized coordinates.
*/
const KEYBOARD_STEP = 0.02;
/** Fine step for keyboard-driven handle resize, in normalized coordinates. */
const KEYBOARD_STEP = 0.01;

/** Coarse step when Shift is held — 10× the fine step. */
const KEYBOARD_STEP_SHIFT = 0.1;

/** Delay before keyboard resize triggers settle (ms). */
const KEYBOARD_SETTLE_DELAY = 500;
Expand Down Expand Up @@ -93,6 +96,7 @@ type RectangleStencilProps = StencilProps;
* @param props.freeformCrop Whether resize handles are shown.
* @param props.stencilTransition CSS transition string for settle animation.
* @param props.cropBounds Maximum crop rect bounds from camera (zoom/rotation-aware).
* @param props.onEscape Called when Escape is pressed on a resize handle.
* @return The rectangle stencil element.
*/
export function RectangleStencil( {
Expand All @@ -106,6 +110,7 @@ export function RectangleStencil( {
freeformCrop = false,
stencilTransition,
cropBounds,
onEscape,
}: RectangleStencilProps ) {
// Use cropBounds from the camera if available, otherwise default to [0,1].
const boundsMinX = cropBounds?.minX ?? 0;
Expand Down Expand Up @@ -192,7 +197,7 @@ export function RectangleStencil( {
ownerDoc.activeElement.blur();
}
// Capture pointer so drag works across iframe boundaries.
const el = event.currentTarget;
const el = event.currentTarget as HTMLButtonElement;
el.setPointerCapture( event.pointerId );

const drag: ResizeDragState = {
Expand Down Expand Up @@ -247,6 +252,11 @@ export function RectangleStencil( {
el.removeEventListener( 'pointerup', onEnd );
el.removeEventListener( 'lostpointercapture', onEnd );
latestHandlersRef.current?.onResizeEnd?.();
// Restore focus to the handle so arrow keys work
// immediately after a mouse drag. Browsers suppress
// :focus-visible after pointer interactions, so the
// focus ring stays hidden until the user presses a key.
el.focus( { preventScroll: true } );
};

el.addEventListener( 'pointermove', onMove );
Expand Down Expand Up @@ -302,12 +312,21 @@ export function RectangleStencil( {
};

/**
* Handle keyboard arrow keys on a resize handle.
* Moves the corresponding edge(s) by KEYBOARD_STEP in normalized space.
* Handle keyboard events on a resize handle.
* Arrow keys resize; Escape returns focus to the canvas.
* Shift multiplies the step size by 10 for coarser movement.
*/
const handleKeyDown = useCallback(
( handle: HandlePosition, event: React.KeyboardEvent ) => {
const key = event.key;

if ( key === 'Escape' ) {
event.preventDefault();
event.stopPropagation();
onEscape?.();
return;
}

if (
key !== 'ArrowUp' &&
key !== 'ArrowDown' &&
Expand All @@ -320,20 +339,22 @@ export function RectangleStencil( {
event.preventDefault();
event.stopPropagation();

const step = event.shiftKey ? KEYBOARD_STEP_SHIFT : KEYBOARD_STEP;

// Determine the normalized delta from the arrow key.
let dx = 0;
let dy = 0;
if ( key === 'ArrowLeft' ) {
dx = -KEYBOARD_STEP;
dx = -step;
}
if ( key === 'ArrowRight' ) {
dx = KEYBOARD_STEP;
dx = step;
}
if ( key === 'ArrowUp' ) {
dy = -KEYBOARD_STEP;
dy = -step;
}
if ( key === 'ArrowDown' ) {
dy = KEYBOARD_STEP;
dy = step;
}

if ( hasLockedRatio ) {
Expand Down Expand Up @@ -382,6 +403,7 @@ export function RectangleStencil( {
computeFreeRect,
onCropChange,
onResizeEnd,
onEscape,
]
);

Expand Down
Loading
Loading