New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keycodes package #7577

Merged
merged 6 commits into from Jun 27, 2018

Conversation

Projects
None yet
2 participants
@youknowriad
Contributor

youknowriad commented Jun 27, 2018

In our effort to split '@wordpress/utils' to smaller targeted packages, I'm extracting "keycodes" to their own package. This would allow us to move forward with the refactoring of the "components" module as a npm ready package.

Tesing instructions

  • Ensure that wp.keycodes == wp.utils.keycodes on the console
  • The differences between the two are the extra warnings triggered by the wp.utils.keycodes utils.

@youknowriad youknowriad self-assigned this Jun 27, 2018

@youknowriad youknowriad requested review from gziolo and WordPress/gutenberg-core Jun 27, 2018

@tofumatt

Looks great to me but I'd like to tweak the package README if that's okay… mind if I sneak a quick commit in? After that I'll approve.

## Installation
Install the module

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

I've seen this in a few other packages and it seems so blunt and unhelpful 😉

Can we say "Install with npm:"?

This comment has been minimized.

@youknowriad

youknowriad Jun 27, 2018

Contributor

I think we should do this in a separate PR to keep consistency between the packages. and yes feel free to add some docs :)

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

Ah, fair point. I'll do that separately, but I'd like to add usage docs to this one 😄

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

Filed #7579, thanks!

@tofumatt

Actually, looks like we need to change the deprecation version.

// keycodes
const wrapKeycodeFunction = ( source, functionName ) => ( ...args ) => {
originalDeprecated( `wp.utils.keycodes.${ functionName }`, {
version: '3.3',

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

Shouldn't this be 3.4? This will be out in 3.2, so we should deprecate after two versions.

@@ -0,0 +1,13 @@
# @wordpress/keycodes
Keycodes utilities for WordPress.

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

Can we say what it's used for, in case someone reading this isn't familiar with what "key codes" means? I have some thoughts… I'll just add them in a commit 😄

@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 27, 2018

Docs added 😄

}
```
## Installation

This comment has been minimized.

@youknowriad

youknowriad Jun 27, 2018

Contributor

I think I'd prefer if we keep the "installation" section before the "usage". It's fine to discuss these but it's important to keep our packages consistent.

This comment has been minimized.

@tofumatt

tofumatt Jun 27, 2018

Member

Totally right, good call. I moved it under "installation"

This comment has been minimized.

@youknowriad

youknowriad Jun 27, 2018

Contributor

Thanks for the change 👍

tofumatt and others added some commits Jun 27, 2018

@youknowriad youknowriad merged commit 6d0fc3b into master Jun 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the add/keycodes-packages branch Jun 27, 2018

@aduth aduth referenced this pull request Jul 2, 2018

Merged

Modal component #6261

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment