Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#54548 closed defect (bug) (fixed)

The Close on Escape "feature" on the admin comments page is annoying

Reported by: bettyjj's profile BettyJJ Owned by: alexstine's profile alexstine
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch
Focuses: ui, accessibility, javascript, administration Cc:

Description

If you reply to a comment from the admin page, whenever you press the esc key, all you have written is lost. Without any warning. What a horrible user experience. I honestly don't understand why this was implemented as a feature in the first place.
This is especially problematic for users with IME, eg. users who type Chinese and Japanese, because esc is used more frequently in the typing process than that of English.
The culprit is in wp-admin/js/edit-comments.js:

$('#replycontent').trigger( 'focus' ).on( 'keyup', function(e){
	if ( e.which == 27 )
		commentReply.revert(); // Close on Escape.
});

I want to propose to just get rid of it. If we want to make it better, maybe show a warning before throwing away what was written, or remember the content so that the content will be still there if the user wants to resume.

I searched and there does not seem to be a recommended way to override core js functions. So I either have to modify the file every time WP updates, or make the change I want into core. So here I'm opening this ticket.

Attachments (6)

Capture d’écran 2022-02-04 à 14.14.43.png (303.8 KB) - added by audrasjb 2 years ago.
After applying the proposed PR
54548.diff (690 bytes) - added by rafiahmedd 2 years ago.
Fix typo in confirm alert
54548.2.diff (770 bytes) - added by afercia 2 years ago.
japanese.gif (145.8 KB) - added by afercia 2 years ago.
Japanese – Kana
chinese.gif (197.2 KB) - added by afercia 2 years ago.
Chinese – Cangjie
54548.3.diff (1.1 KB) - added by afercia 2 years ago.

Download all attachments as: .zip

Change History (21)

#1 @sabernhardt
2 years ago

  • Component changed from General to Comments
  • Focuses accessibility added

#2 @sabernhardt
2 years ago

function originally added in [8720]; the latest code can be found in edit-comments.js

Related: #30979

This ticket was mentioned in Slack in #accessibility by alexstine. View the logs.


2 years ago

#4 @alexstine
2 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.0
  • Owner set to alexstine
  • Status changed from new to accepted

This ticket was mentioned in PR #2277 on WordPress/wordpress-develop by konradyoast.


2 years ago
#5

  • Keywords has-patch added; needs-patch removed

@audrasjb
2 years ago

After applying the proposed PR

#6 @audrasjb
2 years ago

  • Keywords commit added

The pull request looks great to me.
The new confirmation string looks consistent with the other ones in the same file, too.

#7 @sabernhardt
2 years ago

  • Keywords commit removed

Thanks for the patch, but it's not ready yet.

  1. The PR has a typo: should be "reply" instead of "replay" in the second line.
  2. Does the patch improve the interaction for people typing in Chinese or Japanese?
  3. Would removing the close-on-escape feature entirely be a better option?
    $('#replycontent').trigger( 'focus' );
    

@rafiahmedd
2 years ago

Fix typo in confirm alert

#8 @afercia
2 years ago

Testing a bit the latest patch, I noticed a couple issues:

  • The close on Escape feature works also for the 'Quick Edit' textarea so the text of the JavaScript confirmation wouldn't be fully appropriate, as it refers only to 'Reply'.
  • More importantly: when editing multiple comments, there will be multiple JavaScript confirmation dialog. I think this is because of the way the JS event is currently attached to the textarea, which doesn't seem ideal. This would need some more refactoring to be solved, which is something I'd not recommend. To reproduce:
    • Apply the latest patch and build.
    • Reply to (or Quick Edit) a comment.
    • Enter some text.
    • Press the Escape key.
    • Click OK in the JavaScript confirm dialog.
    • Reply to another comment (or even to the same comment).
    • Enter some text.
    • Press the Escape key.
    • Click OK in the JavaScript confirm dialog.
    • At this point, you will get one more confirm dialog.
    • Repeat the steps above a few times and you will get as many confirm dialog as the amount of times you replied to a comment.

Thinking a bit at the patch approach, I'd like to propose a new direction. After all, pressing Escape is generally used in software to 'cancel' the current operation. In this context, it does the same thing the 'Cancel' button (the one placed after the textarea) does. I'm not sure we should change this behavior.

Instead, I'd say the actual problem that needs to be solved is the issue with Input Method Editor (IME) converters. Will submit a patch shortly.

@afercia
2 years ago

#9 follow-up: @afercia
2 years ago

54548.2.diff prevents the textarea from closing when pressing the escape key and while characters are being composed with an Input Method Editor. I can't test this with a real IME so this would need to be tested by someone else.

For some background on this approach, see the discussion on https://core.trac.wordpress.org/ticket/45371

#10 in reply to: ↑ 9 @BettyJJ
2 years ago

Replying to afercia:
Thanks for your effort. I tried your patch and it does not seem to work. It seems that event.isComposing will become false when esc is pressed. See a minimal demo here: https://jsfiddle.net/Lgv7sayq/1/ Tested with the default Chinese IME shipped with Windows 10.

#11 @afercia
2 years ago

@BettyJJ thank you for testing, much appreciated.

I was able to so some testing on macOS. Interestingly, seems to me the IME user interface reacts to keyboard events differently depending on the language. See the animated GIF attached below.

Please consider I have no idea what I'm actually typing while using the IME :) I'm just trying to use it for testing purposes.

Japanese:

  • The first Escape key does NOT close the comment textarea.
  • At that point, isComposing is still true.

Chinese:

  • The first Escape key DOES close the comment textarea.
  • At that point, isComposing is false.

More inconsistencies may occur across browsers and operating systems. Basically, detecting the start of a composition seems reliable. Instead, determining whether a keyup event on the Escape key happens before or after the composition end appears to be highly unreliable.

Given the above, I'd tend to think the most sensible thing to do is to disable the 'Close on Escape' behavior when an IME is in use. Will upload a patch shortly. Any feedback and thoughts are very welcome.

@afercia
2 years ago

Japanese – Kana

@afercia
2 years ago

Chinese – Cangjie

@afercia
2 years ago

#12 @afercia
2 years ago

54548.3.diff disables the 'Close on Escape' behavior when an IME composition has started. Some testing would be appreciated.

#13 @BettyJJ
2 years ago

@afercia Thank you very much. This new patch works for me. :)

#14 @davidbaumwald
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 52951:

Comments: Disable "close on escape" for inline replies when using an IME.

When using an Input Method Editor(IME), pressing escape to perform actions in the IME is common. However, if this was done while replying to a comment, the "close on escape" feature was also triggered which cleared the current textarea and closed it.

This change checks if an IME is in use by binding the compositionstart event to the reply text box and setting a flag if it's triggered. The "close on escape" feature will now only be triggered if this new flag is not set after typing a reply.

Props BettyJJ, sabernhardt, alexstine, konradyoast, audrasjb, rafiahmedd, afercia.
Fixes #54548.

dream-encode commented on PR #2277:


2 years ago
#15

Thanks for the PR! The Trac ticket was resolved in a different way and a fix was merged into core in https://core.trac.wordpress.org/changeset/52951.

Note: See TracTickets for help on using tickets.