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

Hash helper {{#form-for model as |f|}} #215

Closed
wants to merge 3 commits into from

Conversation

diogomafra
Copy link

The future is here! 🎆

Please, don't merge this PR!!

This is a test using the new hash helper available on Ember 2.3. As you can see, it is very simple to support the new syntax.

{{#form-for model as |f|}}
  {{f.input "firstName"}}
  {{f.input "lastName"}}
   <input type="submit">
 {{/form-for}}

@diogomafra
Copy link
Author

For the record, there is an issue related to positional params on Ember 2.3: emberjs/ember.js#12762

We get this error:

Assertion Failed: You cannot specify both a positional param (at position 0) 
and the hash argument `propertyName`.

@bcardarella
Copy link
Contributor

@diogomafra as long as the pre-existing API is not affected then I'm 👍 on this for 1-0-stable. I would prefer to maintain semver compatibility and not break backwards compatibility. It seems from the change in the test this might not be the case?

FWIW, this syntax is the direction I've had in mind for about two years for easy-form 2.0.

@diogomafra
Copy link
Author

@bcardarella It's possible to support both APIs. I just changed the test to demonstrate that the scoped helper is working.

To finish this PR we need:

I'm going to keep this PR here until Ember 2.3 lands, with the issue fixed.

@jcope2013
Copy link

"make it work with previous versions of Ember that don't have the hash helper"

this might help https://github.com/cibernox/ember-hash-helper-polyfill

@diogomafra
Copy link
Author

@jcope2013 this is very nice. Thank you for pointing this addon.

@Serabe
Copy link

Serabe commented Jan 6, 2016

@diogomafra: I think this is not what is causing your problem. I think your problem is in these lines. You can see that knownProperties are copied to the options causing the positional param to already be there.

@Serabe
Copy link

Serabe commented Jan 6, 2016

Sorry, I thought I was writing last comment in emberjs/ember.js#12762

The first this refers to that issue.

Sorry again.

@diogomafra
Copy link
Author

@Serabe the test that is failing does not use this handlebar's keyword, but it has a similar code: https://github.com/diogomafra/ember-easy-form/blob/hash-helper/addon/components/internal-input-for.js#L96

In that code, all "known properties" are removed, "propertyName" among them, so, it's not including this value twice.

I will try to create a failing test on Ember's repo to demonstrate the problem. Then we can be sure it's on Ember and it will be easier to fix.

Thank you!

@Serabe
Copy link

Serabe commented Jan 6, 2016

If you are in the Ember community Slack, ping me to take a deeper look at this, please.

@Serabe
Copy link

Serabe commented Jan 6, 2016

@diogomafra I updated the other issue, but I think this test will pass in 2.3 beta 4 / 2.3.0. There was a fix for the same issue added a day after beta 3 was released.

@diogomafra
Copy link
Author

@Serabe using the canary build instead of beta, it works. Thank you very much for your help.

@Serabe
Copy link

Serabe commented Jan 6, 2016

You're welcome! 👍

@bcardarella
Copy link
Contributor

Closing in favor of #229

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.

4 participants