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

Fix typo in selector #1681

Merged
merged 2 commits into from Sep 10, 2018

Conversation

Projects
None yet
2 participants
@annthurium
Contributor

annthurium commented Sep 6, 2018

Fixes #1675

Requirements

Description of the Change

When I migrated to use the Command component instead of the command registry, I made a typo in the registry target. That means that none of these events were bubbling up to the correct selector, which broke keyboard input on the co author dialog.

I spent a half day trying to write unit tests for these changes to prevent future regressions. However, it ended up being tricker than I anticipated, because:

  • Enzyme doesn't have great support for key events.
  • I don't really understand why this whole proxyKeyCode was necessary to make keyboard events work with react-select in the first place. I'm sure there's a good reason, I just don't know what it is. I reached out to @niik in case he remembers. Knowing this will help me write a better test.
Fix typo in selector
Co-Authored-By: David Wilson <daviwil@users.noreply.github.com>
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Sep 6, 2018

Coverage Status

Coverage increased (+0.08%) to 80.215% when pulling f5c6649 on tt-18-sept-coauthor-bug into e482833 on master.

coveralls commented Sep 6, 2018

Coverage Status

Coverage increased (+0.08%) to 80.215% when pulling f5c6649 on tt-18-sept-coauthor-bug into e482833 on master.

@annthurium annthurium merged commit 51dac31 into master Sep 10, 2018

6 checks passed

ci/circleci: beta Your tests passed on CircleCI!
Details
ci/circleci: dev Your tests passed on CircleCI!
Details
ci/circleci: stable Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 80.215%
Details

@annthurium annthurium deleted the tt-18-sept-coauthor-bug branch Sep 10, 2018

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