Make WordPress Core

Changes between Initial Version and Version 1 of Ticket #60548, comment 1


Ignore:
Timestamp:
02/15/2024 03:45:45 PM (12 months ago)
Author:
afercia
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #60548, comment 1

    initial v1  
    11Some more details:
    22- The `event` used [https://github.com/WordPress/wordpress-develop/blame/7b24083859c37d8ac75b9549f458bc389610eaef/src/js/_enqueues/lib/image-edit.js#L302 here], [https://github.com/WordPress/wordpress-develop/blame/7b24083859c37d8ac75b9549f458bc389610eaef/src/js/_enqueues/lib/image-edit.js#L304 here], and [https://github.com/WordPress/wordpress-develop/blame/7b24083859c37d8ac75b9549f458bc389610eaef/src/js/_enqueues/lib/image-edit.js#L309 here] is the gloabl `window.event`, which is deprecated.
    3 - The `preventDefault()` used [https://github.com/WordPress/wordpress-develop/blame/7b24083859c37d8ac75b9549f458bc389610eaef/src/js/_enqueues/lib/image-edit.js#L309 here] doesn't actually prevents 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.
    4 - 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.
     3- The `preventDefault()` used [https://github.com/WordPress/wordpress-develop/blame/7b24083859c37d8ac75b9549f458bc389610eaef/src/js/_enqueues/lib/image-edit.js#L309 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.
     4- 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.