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

Helper refactor & fixes #252

Merged
merged 3 commits into from
Jul 11, 2013
Merged

Helper refactor & fixes #252

merged 3 commits into from
Jul 11, 2013

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Jul 11, 2013

See issue #246 items 1,2 and 5

Also fixes a bug with the nav helper persisting the active item

Thought I'd put this up as a PR and see if anyone has any feedback before I merge it tomorrow

- updated navigation and pagination helpers to use SafeString
- nav and pagination don't need triple taches any more
- nav tests updated, and renamed to match helper name
- fixed a bug whereby once you visit the homepage the homepage menu item is always marked as the active page
- this was due to passing the config object being done by reference rather than by value, and therefore setting the selected item was persisted.
- moved template logic out of individual helpers and into Ghost
- simplified template-driven helpers into closures which maintain the context of handlebars
- with handlebars context we have access to data, so don't need to pass data in
- check data to test that it is a simple object and not a function
- moved helpers back into index.js
- provided tests for both template functions in ghost and the nav helper so we are back to where we were
ErisDS added a commit that referenced this pull request Jul 11, 2013
@ErisDS ErisDS merged commit f3c3323 into TryGhost:master Jul 11, 2013
@ErisDS ErisDS deleted the helpers branch July 11, 2013 16:58
morficus pushed a commit to morficus/Ghost that referenced this pull request Sep 4, 2014
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.

1 participant