Make WordPress Core

Opened 18 months ago

Closed 15 months ago

Last modified 15 months 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 16 months ago.
After applying the proposed PR
54548.diff (690 bytes) - added by rafiahmedd 16 months ago.
Fix typo in confirm alert
54548.2.diff (770 bytes) - added by afercia 15 months ago.
japanese.gif (145.8 KB) - added by afercia 15 months ago.
Japanese – Kana
chinese.gif (197.2 KB) - added by afercia 15 months ago.
Chinese – Cangjie
54548.3.diff (1.1 KB) - added by afercia 15 months ago.

Download all attachments as: .zip

Change History (21)

#1 @sabernhardt
18 months ago

  • Component changed from General to Comments
  • Focuses accessibility added

#2 @sabernhardt
18 months 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.


18 months ago

#4 @alexstine
18 months 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.


16 months ago
#5

  • Keywords has-patch added; needs-patch removed

@audrasjb
16 months ago

After applying the proposed PR

#6 @audrasjb
16 months 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
16 months 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
16 months ago

Fix typo in confirm alert

#8 @afercia
15 months 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
15 months ago

#9 follow-up: @afercia
15 months 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
15 months 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
15 months 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
15 months ago

Japanese – Kana

@afercia
15 months ago

Chinese – Cangjie

@afercia
15 months ago

#12 @afercia
15 months ago

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

#13 @BettyJJ
15 months ago

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

#14 @davidbaumwald
15 months 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:


15 months 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.