Opened 9 years ago
Closed 9 years ago
#36316 closed defect (bug) (fixed)
Image editor: improve form validation errors handling
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | has-patch has-screenshots |
Focuses: | ui, javascript | Cc: |
Description
When editing images and entering values for Scale, Crop ratio/selection in the Image Editor form, the check for entered values could be improved a bit. I've noticed this because a plugin was giving a JavaScript error and as a consequence, when entering new scale width and height, in the input field a "NaN" (literally) value appeared :)
Even with no plugin activated and no JS errors, when entering non-numbers, "Nan" is displayed to users, see below:
Users would probably be a bit confused by seeing "NaN", I'd say it could be improved a bit. Worth noting the same thing doesn't happen in the Crop input fields: when entering non-number values, the field gets emptied.
Not pretending to completely refactor the code behind the Image Editor, which is a bit old but still works. I'd propose to keep things simple and:
- use the same check for a numeric value used on the Crop fields on all the other fields: don't display "NaN", just empty the field
- remove the inline script that runs the initialisation of the Image Editor and call it after the Editor UI is fully ready; this would probably help a bit to mitigate lack of data necessary for a correct initialisation
(technical detail: the plugin JS error caused the imageEdit.hold
object to be empty because not initialised)
Attachments (2)
Change History (14)
#4
@
9 years ago
- remove the inline script that runs the initialisation of the Image Editor
Looks like I forgot this part :) Patch refreshed.
This ticket was mentioned in Slack in #core-editor by afercia. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-images by afercia. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
9 years ago
#9
@
9 years ago
Note: about the inline event handlers, the patch just uses the existing pattern. Changing them would be good but maybe a bit out of the scope of this ticket. As mentioned in the ticket description:
Not pretending to completely refactor the code behind the Image Editor, which is a bit old but still works
This ticket was mentioned in Slack in #core-editor by afercia. View the logs.
9 years ago
#11
@
9 years ago
It was suggested to use type="number"
for the input fields, it would probably be a good thing even if not all browsers prevent to type non-numerical characters in those fields. By the way, worth mentioning the onkeypress
event in imageEdit.init()
currently triggers a function that doesn't allow to use the arrow keys on those fields when using Firefox. Worth reminding onkeypress
works differently in different browsers and in Firefox it fires when using the arrow keys so the fields will be blurred because of:
if ( 36 < k && k < 41 ) { $(this).blur(); }
Thinking this part and all the events in the Image Editor should be completely reviewed, not sure if in this ticket.
First pass. Any thoughts more than welcome.