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

Switch to ember input helper #64

Closed
wants to merge 1 commit into from
Closed

Switch to ember input helper #64

wants to merge 1 commit into from

Conversation

RobbieTheWagner
Copy link
Collaborator

@RobbieTheWagner RobbieTheWagner commented Apr 3, 2017

Just kidding, this doesn't fix the issue, but it is a step in the right direction.

@knownasilya
Copy link
Owner

knownasilya commented Apr 4, 2017

We probably want to follow something like https://ember-twiddle.com/ac4ea1deedfcb7cb2b13a40c3f166932?openFiles=templates.application.hbs%2C

Might be worthwhile nailing down the failing test case, so we know it's solved.

@RobbieTheWagner
Copy link
Collaborator Author

@knownasilya this doesn't have a failing test case, just switched to the input helper, which seems to work fine. This is not part of the refactor we discussed yet.

@knownasilya knownasilya closed this Apr 6, 2017
@knownasilya
Copy link
Owner

@knownasilya
Copy link
Owner

knownasilya commented Apr 6, 2017

Also see https://github.com/knownasilya/ember-toggle/blob/simplify/tests/dummy/app/templates/application.hbs#L184 If everything looks good, I'll probably be doing a major release.

@RobbieTheWagner
Copy link
Collaborator Author

@knownasilya tentatively looks okay. I like all the code deletion 😄 .

I think it should still use the ember input helper. Is there a reason we couldn't merge this PR?

Let me test out the new branch and if everything works, would you mind me doing a bit of cleanup, like using the input helper and submitting a PR or are you pretty set on things the way they are?

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.

2 participants