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

Solve for "i" not defined AND adding in a "is_current" property for each... #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

csprocket777
Copy link

... page in Pages

So I'm finding this class useful. However when I implemented it, I found that I received an error stating that "i" was not defined. This simply defines the variable in the for loop.

Also, I thought it was a nice shortcut to add an "is_current" property to each page that references the "currentPage" property of the mixin.

…ach page in Pages

So I'm finding this class useful. However when I implemented it, I found that I received an error stating that "i" was not defined. This simply defines the variable in the for loop.

Also, I thought it was a nice shortcut to add an "is_current" property to each page that references the "currentPage" property of the mixin.
@toranb
Copy link
Owner

toranb commented Nov 3, 2013

Awesome! Would you be able to writing a jasmine test (in the current test file) to show this broken without the fix you applied?

Thank you in advance

@csprocket777
Copy link
Author

Unfortunately I have NO experience with Jasmine. Is this something I would need to do before you accept the PR?

-Chuck

On Nov 2, 2013, at 8:19 PM, Toran Billups notifications@github.com wrote:

Awesome! Would you be able to writing a jasmine test (in the current test file) to show this broken without the fix you applied?

Thank you in advance


Reply to this email directly or view it on GitHub.

@toranb
Copy link
Owner

toranb commented Nov 5, 2013

To be completely honest this example was built with the sole purpose of showing how to do pagination with ember and how to test it in isolation. Jasmine itself doesn't have a steep learning curve and this would be a great learning opportunity (as this isn't production code for anyone).

How do the tests run currently on your machine with these changes?

If you first install everything using npm install (from the project root), you can run all the tests like so

node node_modules/jasmine-phantom-node/bin/jasmine-phantom-node example/static

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