WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 3 months ago

Last modified 11 days ago

#41545 closed enhancement (fixed)

Allow cmd/ctrl-enter to submit comment forms

Reported by: helen Owned by: adamsilverstein
Milestone: 5.3 Priority: normal
Severity: minor Version:
Component: Comments Keywords: good-first-bug has-patch commit
Focuses: javascript Cc:
PR Number:

Description

It's become fairly common for comment/reply forms on the web to be submitted with cmd/ctrl-enter. Some quick examples include Twitter and GitHub, neither of which tells you as far as I've seen but rather just lets you discover it works. I think it'd be a nice enhancement for comment forms in both the admin and front end to submit this way. In particular, it would make managing and responding to comments a little smoother.

Attachments (7)

41545.diff (59.2 KB) - added by Lindstromer 2 years ago.
Added support for Ctrl/Cmd+Enter to submit replies in wp-admin and frontend. + code style for the specified js-files.
41545.2.diff (2.6 KB) - added by Lindstromer 2 years ago.
PHPStorm bugged with code style messing up files in previous diff. Also added html-file that was missed.
41545.3.diff (2.6 KB) - added by xyfi 15 months ago.
41545.4.diff (768 bytes) - added by adamsilverstein 9 months ago.
41545.5.diff (786 bytes) - added by splitti 5 months ago.
Updated patch to enable CMD+enter as well
Comment Submission Failure 2019-06-22 17-27-06.jpg (24.7 KB) - added by adamsilverstein 5 months ago.
41545.6.diff (982 bytes) - added by adamsilverstein 5 months ago.

Download all attachments as: .zip

Change History (40)

#1 @Lindstromer
2 years ago

I'll try and submit a patch for it tomorrow, the wp-admin part is solved but front end needs a little more work.

@Lindstromer
2 years ago

Added support for Ctrl/Cmd+Enter to submit replies in wp-admin and frontend. + code style for the specified js-files.

#2 @Lindstromer
2 years ago

  • Keywords needs-testing has-patch added; needs-patch removed

The patch is functioning in Firefox, Safari and Chrome on OS X when using Ctrl or Cmd + Enter. Needs testing on different operating systems to see that it picks upp the key kombination as expected.

The solution isn't to clean for the frontend part, where I had to workaround the fact that submit() is overwritten by the submit-button id.

An alternative would be to refactor the whole form output and give it a better id but I'm guessing that it could create issues.

@Lindstromer
2 years ago

PHPStorm bugged with code style messing up files in previous diff. Also added html-file that was missed.

#3 @DrewAPicture
22 months ago

  • Owner set to lindstromer
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

#4 @adamsilverstein
18 months ago

  • Owner changed from lindstromer to adamsilverstein

#5 @adamsilverstein
18 months ago

  • Milestone changed from Awaiting Review to 4.9.7

#6 @desrosj
17 months ago

  • Keywords needs-refresh added

Patch needs a refresh to work with the new JavaScript file locations/build process.

#7 @ocean90
17 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

This ticket was mentioned in Slack in #core by jon_bossenger. View the logs.


17 months ago

This ticket was mentioned in Slack in #core by joshuawold. View the logs.


17 months ago

#10 @pento
16 months ago

@adamsilverstein: Are you likely to be able to review this in time for 4.9.8 beta 1?

#11 @adamsilverstein
16 months ago

@pento apologies i've been travelling and just saw your note. I will look at this today.

#12 @adamsilverstein
16 months ago

  • Milestone changed from 4.9.8 to 4.9.9

I took a look at this yesterday and I think it needs a little more work to be ready:

  • Let's attach the event listener in JavaScript instead of in the HTML of the element itself. This makes it easier to tell where the event is getting called from and is the common pattern we use across most of core.
  • Switch to the keyup event instead of keydown which I think is better from a usability perspective?
  • Can we find/submit the original form instead of the current approach of creating a form?

Punting to 4.9.9 while we finish up these details.

@xyfi
15 months ago

#13 @xyfi
15 months ago

I have brought the patch created by @Lindstromer up to date with the current core code. I have not yet come around to implementing @adamsilverstein 's requested changes.

#14 @pento
14 months ago

  • Milestone changed from 4.9.9 to 5.0.1

#15 @pento
11 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#16 @pento
11 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#17 @audrasjb
11 months ago

  • Milestone changed from 5.0.3 to 5.1

Hello,

5.0.3 is going to be released in a couple of weeks.

It doesn't appear this ticket can be handled in the next couple of weeks (needs some work and review). Let's address it in 5.1 which is coming in February. Feel free to ask for changing the milestone if you think this issue can be quickly resolved.

Cheers,

Jb

#18 @pento
11 months ago

  • Milestone changed from 5.1 to 5.2

Patch needs refreshing.

#19 @adamsilverstein
9 months ago

In 41545.4.diff:

  • Refactor to add the event listener to comment-reply init, so its set on a per form basis.
  • Inline comment explaining why we use a click() action on the button vs. calling submit() on the form

#20 follow-up: @adamsilverstein
9 months ago

@xyfi @Lindstromer can you give 41545.4.diff a test?

#21 in reply to: ↑ 20 @Lindstromer
9 months ago

Replying to adamsilverstein:

@xyfi @Lindstromer can you give 41545.4.diff a test?

Thanks for the ping. Give me a day or two and I'll look into it.

#22 @adamsilverstein
9 months ago

Thanks @Lindstromer! Note if you haven't tested a patch recently: you now need to build the javascript (try npm install && grunt build) and run WordPress from the build folder (possible this requirement may have changed) - see https://make.wordpress.org/core/2018/05/16/preparing-wordpress-for-a-javascript-future-part-1-build-step-and-folder-reorganization/ for the original post explaining the change.

#23 follow-up: @desrosj
8 months ago

  • Milestone changed from 5.2 to 5.3

@Lindstromer have you been able to test 41545.4.diff?

I am going to punt this to 5.3 since this still needs feedback. @adamsilverstein if you feel confident in this for 5.2, feel free to move it back.

#24 in reply to: ↑ 23 @Lindstromer
8 months ago

Replying to desrosj:

@Lindstromer have you been able to test 41545.4.diff?

I am going to punt this to 5.3 since this still needs feedback. @adamsilverstein if you feel confident in this for 5.2, feel free to move it back.

Hi @desrosj , I'm sorry I haven't had time to work with this at all. I'll give it a try soon.

This ticket was mentioned in Slack in #core-js by splitti. View the logs.


5 months ago

#26 @splitti
5 months ago

I tested the patch and noticed that the e.metaKey is not set for Cmd in the keyup events, but keydown works. Even though this might fire more than once, the click event should be processed first and that will submit the form. I don't think switching to keydown is a problem, but will enable Cmd for submitting as is desired in this ticket.

If we want to just enable Ctrl + Enter then I'd be happy to rescind my updated diff and I'd say we're ready to land this patch.

@splitti
5 months ago

Updated patch to enable CMD+enter as well

#27 @adamsilverstein
5 months ago

One issue I noticed with 41545.5.diff is that if i hold ctrl and double tap the enter key, the form is submitted twice and I see the 'duplicate entry' error Comment Submission Failure 2019-06-22 17-27-06.jpg.

#28 @adamsilverstein
5 months ago

In 41545.6.diff:

  • remove the event handler when submitting form to avoid duplicate submissions

#29 @adamsilverstein
3 months ago

  • Keywords commit added; needs-testing needs-refresh removed

#30 @adamsilverstein
3 months ago

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

In 45790:

Comments: enable typing cmd/ctrl-enter to submit comment forms.

Add a key handler on the comment form that detects the cmd/ctrl-enter key press and submits the comment form.

Props xyfi, Lindstromer, helen, splitti.
Fixes #41545.

#31 @SergeyBiryukov
3 months ago

In 45792:

Coding Standards: Fix JSHint error in [45790].

See #41545.

#32 @SergeyBiryukov
11 days ago

In 46700:

Comments: Check if comment form element exists before adding a key handler to detect the cmd/ctrl-enter key press.

Follow-up to [45790].

Props raamdev.
Fixes #48543. See #41545.

#33 @whyisjake
11 days ago

In 46706:

Comments: Check if comment form element exists before adding a key handler to detect the cmd/ctrl-enter key press.

Brings [46700] to the 5.3 branch.

Follow-up to [45790].
Props raamdev.
Fixes #48543. See #41545.

Note: See TracTickets for help on using tickets.