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

fixes the 'invalid input type' check (passedElement) #498

Merged

Conversation

jhou
Copy link
Contributor

@jhou jhou commented Jan 20, 2019

Description

in the constructor, the second check for passedElement ( https://github.com/jshjohnson/Choices/blob/9c001487bacad7588e92f8b6d54880b88bf21fc6/src/scripts/choices.js#L104 ) should be checking this.passedElement, that's what's getting set by the WrappedInput or WrappedSelect.

Motivation and Context

If the user makes a mistake, and tries to instantiate Choices on a hidden input (a bug in my code caused that to happen), it causes a javascript exception way down the execution path, instead of giving the intended warning.

How Has This Been Tested?

Manual testing so far. I tried to write tests in text.spec.js, but I am not quite experienced enough with Cypress to get to to work, I guess. I was attempting to use cy.spy(console, 'error') to make sure the right console error was raised, but I couldn't seem to get it working - the expect(spy).to.be.called always fails.

So maybe this needs some tests added, too, though it's not as critical as my last PR, so i'll let you make the call.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@jshjohnson jshjohnson merged commit 48b74a9 into Choices-js:master Jan 21, 2019
@jshjohnson
Copy link
Collaborator

👌 good spot, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants