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

[BUGFIX beta] Read values of action helper parameters #12764

Merged
merged 1 commit into from
Jan 4, 2016

Conversation

Serabe
Copy link
Member

@Serabe Serabe commented Dec 30, 2015

This change makes helper action read values when the action is executed
instead of just using the value. This worked as far as the value was not bound.

Fixes #12740

@@ -1002,6 +1085,29 @@ QUnit.test('should respect preventDefault=false option if provided', function()
equal(event.isDefaultPrevented(), false, 'should not preventDefault');
});

QUnit.test('should respect preventDefault=false option if provided bound', function() {
view = EmberView.create({
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I believe you can use an Ember.Component here and set the targetObject to be the controller. I am wary of adding new tests that use EmberView directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I agreed on meeting @rwjblue after 2.3 was released to plan moving tests away from Ember.View. While new tests suites are easy to start with Ember.Component, most test files using views have already their setup and teardown based on that.

I rather write this with Ember.View and start working on a plan to migrate tests one complete file at a time.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with both here. 😺

Lets land to match the tests in the surrounding area, and then work to actually fix the test suite.

@mixonic
Copy link
Sponsor Member

mixonic commented Dec 30, 2015

@Serabe if we add support for bubbles=someBoundProp, we should also support that bound value changing. Can you assert someBoundProp can be changed for allowedKeys, bubble, and preventDefault? The current tests only assert the initial value.

This change makes helper `action` `read` values when the action is executed
instead of just using the value. This worked as far as the value was not bound.

Fixes emberjs#12740
@Serabe
Copy link
Member Author

Serabe commented Dec 30, 2015

Added!

rwjblue added a commit that referenced this pull request Jan 4, 2016
[BUGFIX beta] Read values of `action` helper parameters
@rwjblue rwjblue merged commit 90d51dc into emberjs:master Jan 4, 2016
@rwjblue
Copy link
Member

rwjblue commented Jan 4, 2016

Thank you @Serabe!

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

3 participants