Skip to content
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

Keyboard Shortcuts: Replace store name string with exposed store definition #27355

Conversation

bosconian-dynamics
Copy link
Contributor

@bosconian-dynamics bosconian-dynamics commented Nov 29, 2020

Description

Replaces references to the core/keyboard-shortcuts store name string with the store definition object, adding imports
where necessary.

Addresses #27088.

How has this been tested?

npm run test-unit on a Windows 10 machine.

The packages/env/lib/config/test/config.js suite failed 11 tests for reasons unrelated to this commit - namely, seemingly the Windows 10 environment and Windows-style file-paths. I will set up a linux environment for testing moving forward.

Types of changes

Non-breaking refactor replacing string store names to store definition objects. Added named imports where necessary.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

definition

Replaces references to the `core/keyboard-shortcuts`
store name string with the store definition object, adding `import`s
where necessary.

Addresses WordPress#27088.
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 29, 2020
@bosconian-dynamics
Copy link
Contributor Author

bosconian-dynamics commented Nov 29, 2020

One of the failing E2E tests seems to be related to #27229 but I'm not sure about the other two.

It's looking like Windows really isn't suited to Gutenberg development... I'm going to get my linux environment together tomorrow and try to explore this further - but I'd certainly appreciate any nudges in the right direction!

@gziolo gziolo added [Package] Keyboard Shortcuts /packages/keyboard-shortcuts [Type] Code Quality Issues or PRs that relate to code quality labels Nov 29, 2020
@gziolo
Copy link
Member

gziolo commented Nov 29, 2020

Thank you for opening this PR. Changes look good, I'll have a closer look tomorrow 👍

The packages/env/lib/config/test/config.js suite failed 11 tests for reasons unrelated to this commit - namely, seemingly the Windows 10 environment and Windows-style file-paths. I will set up a linux environment for testing moving forward.

I don't see any reason why Window 10 shouldn't be supported. It would be great to file an issue with errors you had.

Failing e2e tests are confusing, there is PR that tries to fix 2 of them: #27347.

@gziolo gziolo merged commit 0fce044 into WordPress:master Nov 30, 2020
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 30, 2020
@bosconian-dynamics bosconian-dynamics deleted the refactor/replace-keyboardshortcut-store-keys-with-store-definitions branch November 30, 2020 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Package] Keyboard Shortcuts /packages/keyboard-shortcuts [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants