Make WordPress Core

Opened 12 years ago

Closed 4 years ago

Last modified 4 years ago

#19489 closed enhancement (maybelater)

Consider updating jquery.hotkeys plugin

Reported by: garyj's profile GaryJ Owned by:
Milestone: Priority: low
Severity: normal Version: 3.3
Component: External Libraries Keywords: needs-patch
Focuses: javascript Cc:

Description

The current version of the jquery.hotkeys plugin in trunk is "(beta)(0.0.3)", where as the latest version (as linked to from the credits screen) is 0.8.

Incidentally, this 0.8 version from tzuryby is credited as being authored by John Resig (and indeed, he appears to have his own repo for it.

Change History (13)

#1 follow-up: @kurtpayne
12 years ago

  • Cc kpayne@… added

It looks like there was a version jump from 0.03 to 0.7. The later version looks like a complete rewrite. As of 0.8, the project was moved to github. I can't find anything about releases 0.4 through 0.6.

The API has changed, too. API as of 0.03:

$.hotkeys.add('Ctrl+C', {options ... }, function() {...});

API as of 0.8:

$(document).bind('keyup', 'Ctrl+C', function() {...});

I tested this on a local installation and was getting some very odd behavior similar to this comment. After some investigation, I wasn't able to discern the root cause.

#2 @GaryJ
12 years ago

For jQuery 1.7+ you should be using on() instead of bind(), but I doubt that would fix the experience you're getting with it.

As both tzuryby and jeresig seem not to be addressing the outstanding issues, it may be worth following up on one of the recently active (one committed yesterday for instance) forks as visible from https://github.com/jeresig/jquery.hotkeys/network.

#4 in reply to: ↑ 1 @SergeyBiryukov
12 years ago

Replying to kurtpayne:

API as of 0.8:

$(document).bind('keyup', 'Ctrl+C', function() {...});

According to the readme and this comment, the correct syntax is this:

$(document).bind('keydown.ctrl_a', fn);

#5 @azaozz
12 years ago

  • Milestone changed from Awaiting Review to Future Release

Perhaps it would be best to also check/refactor table-hotkeys.js (our implementation of the plugin) too. A good initial patch is at http://core.trac.wordpress.org/attachment/ticket/20885/20885.patch.

#6 @vincentastolfi
11 years ago

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

I have been having trouble getting this plugin to work properly -- it kept binding all the keys at once no matter what key I set. Because of this, hitting any key would trigger my alert "hello." It took two days to find what was wrong, but this code below should work for everyone, where 'S' is the key you chose to keydown (or up) upon:

jQuery(document).bind('keydown', function (e){
	if (String.fromCharCode(e.keyCode) == 'S') {
		alert('hello');
		e.preventDefault();
		return false;
	}
});

Hope this is helpful to someone!

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#7 @helen
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#8 @chriscct7
8 years ago

  • Keywords needs-patch added

#9 @desrosj
4 years ago

  • Focuses javascript added
  • Keywords close added
  • Milestone set to 5.4

I'm marking this as a low priority, and as a candidate to close for a few reasons:

  • This is a super old library that, even in forked repositories, appears to be no longer actively maintained.
  • In core, this only seems to be used for comments.
  • There are a few lines commented out for browser compatibility reasons (see #20885).
  • In the future, comments may actually become blocks/block editors.
  • There are better, more modern ways to accomplish this.

But, let's try to at least look at this and close this out in 5.4 by either:

There seem to be some compatibility changes that need to be made to update to the "latest" version. If this requires anything more than a trivial level of effort, I vote to leave it as is.

Last edited 4 years ago by desrosj (previous) (diff)

#10 @davidbaumwald
4 years ago

@desrosj Revisting this one, do you still feel this is a close candidate at this point? I don't see any recent movement here, and 5.4 Beta 1 is closing in.

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


4 years ago

#12 @desrosj
4 years ago

  • Keywords close removed
  • Milestone 5.4 deleted
  • Resolution set to maybelater
  • Status changed from reopened to closed

I am going to close this one out. If someone wants to spend the time working on this it can be reopened.

But, since there are more modern ways to accomplish this, and it is not heavily utilized by Core, I think it can be left as is.

#13 @desrosj
4 years ago

  • Priority changed from normal to low
Note: See TracTickets for help on using tickets.