Image editor: improve the browsePopup function

[afercia]

While auditing all the remaining jQuery deprecations still to address in core, I noticed a couple things that offer room for improvements in the browsePopup function of the core image editor.

1. The browsePopup function uses the window.event property, which is deprecated and should not be used. This is the global event. Instead, the event should be passed as a function parameter. Aside: instead of using inline events, it could have been better to consider a more modern approach.
2. When using the keyboard to navigate the items in the 'Image rotation' dropdown menu, the page scrolls. See attached animated GIF. There's some code in place to prevent page scrolling you may need to reduce your viewport height.

See [55919] / #50523

[afercia]

Some more details:

• The event used ​here, ​here, and ​here is the gloabl window.event, which is deprecated.
• The preventDefault() used ​here doesn't actually prevent the page scrolling default action because the event in use is keyup which fires when a key is released. At that point the page scroll already occurred when pressing the key.
• As a minor improvement, I'd suggest to rename the $target variable as that's not a jQuery object wrapping a DOM object. It is either false or the value returned by jQuery get() which is the DOM object. Traditionally, the core JavaScript prefixes variable names with $ as a convention for jQuery objects while this is a 'native' DOM object.

[deepakvijayan]

Adjusting the event handling from onkeyup to onkeydown and passing the event object (event) as a parameter to the browsePopup function should help resolve the issues right?

Using onkeydown would indeed prevent the page from scrolling down as well.

For example inside image-edit.php if we change the following

<button type="button" class="imgedit-rleft button" onkeyup="imageEdit.browsePopup(this)" onclick="imageEdit.rotate( 90, <?php echo "$post_id, '$nonce'"; ?>, this)" onblur="imageEdit.monitorPopup()"><?php esc_html_e( 'Rotate 90&deg; left' ); ?></button>

to

<button type="button" class="imgedit-rleft button" onkeydown="imageEdit.browsePopup(event, this)" onclick="imageEdit.rotate( 90, <?php echo "$post_id, '$nonce'"; ?>, this)" onblur="imageEdit.monitorPopup()"><?php esc_html_e( 'Rotate 90&deg; left' ); ?></button>

similarly for all the buttons should help.

Also browsePopup could receive the event attribute like below

browsePopup: function (event, el) {
...

Also, we won't have to worry about depending on the global window.event.

I did test this out on my local environment and seems to be working fine.

[deepakvijayan]

after applying the fixes

[afercia]

Adjusting the event handling from onkeyup to onkeydown and passing the event object (event) as a parameter to the browsePopup function should help resolve the issues right?

yes, either that or we would need to add a keydown event for the only purpose to prevent scrolling. It depends whether keeping the interaction with the menu on keyup is desired or not. I'd defer to @joedolson for a decision about the preferred interaction.

[joedolson]

Re: "Aside: instead of using inline events, it could have been better to consider a more modern approach. "

I didn't see any reason to have everything else in inline events with just this event being executed differently from the rest of the image editor, nor did I see it as worthwhile to refactor the entire image editor. So, I pretty much stand by that approach in this case, even though it's far from ideal.

[nirajgirixd]

I haven't seen any PR raised for this issue. If it is still valid, shall I raise a PR for the fix?

cc: @afercia @joedolson

[afercia]

@nirajgirixd sure please do feel free to go ahead, thanks!

## Description

This PR addresses the scrolling issue encountered when using the keyboard to navigate different options in the image editor.

## Changes Made

• Replaced onkeyup event with onkeydown event.

## Why?

When using the keyboard to navigate the items in the 'Image rotation' dropdown menu, the page scrolls.

## Screencast

​https://github.com/WordPress/wordpress-develop/assets/51747980/fe1f8e04-0a3d-462a-8d10-00ca4ecdaae7

---

## Trac ticket: https://core.trac.wordpress.org/ticket/60548

[joedolson]

Update PR to include JS changes & pass event.

[joedolson]

The updated patch incorporates the changes suggested by @afercia and @deepakvijayan for passing events and updating formatting.

Tested, works as expected for me: marking as needs testing.

Testing instructions:

Navigate to the media and edit an image.
Navigate the 'Image Rotation' menu using the tab and arrow keys. Observe that there is no unexpected shift of viewport position while browsing.

[antpb]

This is working good in my testing. Validated the before and after scroll event viewport behavior. Code looks good. Let's ship it :)

Trac ticket: https://core.trac.wordpress.org/ticket/60548

[joedolson]

In 58946:

Media: Fix viewport scrolling and code style in image rotation.

Change browsePopup to use onkeydown, pass the event parameter from the calling control, and adjust variable naming style.

The browsePopup method used for the image rotation menu used onkeyup to trigger events, which prevented capturing browser scroll actions with arrows occurring on onkeydown.

Props afercia, deepakvijayan, nirajgirixd, joedolson, antpb.
Fixes #60548.

In r58946