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

Fixed double sending of onInvalid action #406

Closed

Conversation

shoxter
Copy link
Contributor

@shoxter shoxter commented Jun 29, 2016

Cleaned up the code a little to use the local var.

Main point: Fix specifically when onInvalid sends.

Because onInvalid is triggered when !== to lastIsInvalid, null and false values triggered two separate onInvalid calls.

As far as validity goes, these two are one in the same. Added !! to the if so that if the case is null !== false, it will now be false !== false and not send the extra onInvalid call.

@SaladFork
Copy link
Contributor

SaladFork commented Jun 29, 2016

Does it make sense for the action to send up the value (null, false, true) or the boolean coercion of it (false, true)?

@shoxter
Copy link
Contributor Author

shoxter commented Jun 29, 2016

We can do either. That's up to you guys. Both can work for me.

I think the boolean coercion would make it simpler for people not reading the source code of the paper-input; however, differentiating null from false in the component helps tell whether it's invalid prior to being touched or after.

@shoxter shoxter closed this Jun 29, 2016
@shoxter
Copy link
Contributor Author

shoxter commented Jun 29, 2016

Implementing a better solution and then I'll reopen.

@shoxter shoxter reopened this Jun 29, 2016
@shoxter
Copy link
Contributor Author

shoxter commented Jun 29, 2016

Not sure why the tests fail? They pass with ember test...

shoxter and others added 6 commits June 29, 2016 18:15
- Remove ember-try (it is included with ember-cli)
- Exclude tests/helpers/module-for-acceptance.js from jscs
- Other dependency updates per ember-cli addon diffs
- Travis config update per ember-cli addon diffs
@miguelcobain
Copy link
Collaborator

What does this include the paper-form component, just like #408?
Can't we just consider one over the other?

export default Ember.Component.extend({
tagName: '',
attributeBindings: ['style'],
style: "width:100%",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're only using single quotes. This should lead to a failing jscs error in the tests.

@shoxter shoxter closed this Jul 4, 2016
shoxter and others added 9 commits July 4, 2016 21:19
* Changed the way the paper-input component and input element share values. With this, values are consistent between the element and component at all times
* Added comments. Moved action. Simplified didRender
@shoxter shoxter reopened this Jul 5, 2016
@shoxter shoxter closed this Jul 5, 2016
@shoxter shoxter deleted the fix-input-validation branch July 5, 2016 17:02
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.

5 participants