Modal: Nuance outside interactions

[stokesman]

What?

Changes interactions on the overlay:

• To not dismiss until clicked, thereby allowing to cancel dismissal by dragging back over the modal before releasing.
• To permit opening the context menu without dismissal

Why?

Some portion of the audience will appreciate these nuances and they don't otherwise negatively affect any UX.

How?

Uses pointer event handlers and:

• at pointerdown if the overlay was pressed with the primary pointer then it’s tracked.
• at pointerup determines if the modal should be dismissed based on whether the overlay was the release target and whether the pointer (and button for mice) was primary.

Testing Instructions

1. Open a modal.
2. Confirm ability to open the context menu with an alt click on the overlay without dismissing the modal.
3. Confirm ability to press the overlay without dismissing the modal.
4. Confirm ability to cancel dismissal by dragging back over the modal before releasing the press.

Testing Instructions for Keyboard

The changes in question aren’t keyboard applicable.

Screenshots or screencast

[mirka]

I might be missing something obvious, but what stops us from simply doing this?

Suggested change

	onPointerUp: ( { target, isPrimary, button } ) => {
	const isSameTarget = target === pressTarget;
	pressTarget = null;
	if ( isPrimary && button === 0 && isSameTarget ) onRequestClose();
	},
	onClick: ( event ) => {
	if ( event.target === event.currentTarget ) {
	onRequestClose();
	}
	},

[stokesman]

It wasn't immediately obvious to me. I tried that first and it works except for supporting the ability to drag back over the modal to cancel the click. When mousedown is on the overlay and the mouseup is on the modal that's still a click from the perspective of the overlay (and target is currentTarget).

[mirka]

Oh I see, good catch. Could we add a code comment about that too? I'm sure others will wonder the same.

Unfortunately this doesn't seem to be accurately testable in Jest, so I'll take note of it as a potential test to add when we set up Playwright tests for components.

[mirka]

I think this is a pretty important improvement, let's move forward with it!

[stokesman]

Thanks for reviewing Lena! I added a comment as suggested. I also removed the condition concerning isPrimary. I'd been supposing it might be a good to cover a case on multi input devices (think touch screen laptop with mouse) but I think that’s over doing it and I don’t have a device like that for testing anyway.