From 53fe5e3ba3e26b9320ffefa7b0b27f65abad1619 Mon Sep 17 00:00:00 2001 From: ErisDS Date: Tue, 9 Jul 2013 16:55:23 +0100 Subject: [PATCH 1/3] HTML helpers work with double taches - issue #246 item 1. - 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 --- core/frontend/helpers/navigation.js | 7 ++----- core/frontend/helpers/paginate.js | 8 +++----- ...ostNav_spec.js => frontend_helpers_navigation_spec.js} | 2 +- 3 files changed, 6 insertions(+), 11 deletions(-) rename core/test/ghost/{frontend_helpers_ghostNav_spec.js => frontend_helpers_navigation_spec.js} (97%) diff --git a/core/frontend/helpers/navigation.js b/core/frontend/helpers/navigation.js index f88573d4403a..0ed7a4a52698 100644 --- a/core/frontend/helpers/navigation.js +++ b/core/frontend/helpers/navigation.js @@ -3,6 +3,7 @@ var fs = require('fs'), _ = require('underscore'), handlebars = require('express-hbs').handlebars, nodefn = require('when/node/function'), + NavHelper; NavHelper = function (navTemplate) { @@ -29,11 +30,7 @@ NavHelper.prototype.compileTemplate = function (templatePath) { }; NavHelper.prototype.renderNavItems = function (navItems) { - var output; - - output = this.navTemplateFunc({links: navItems}); - - return output; + return new handlebars.SafeString(this.navTemplateFunc({links: navItems})); }; // A static helper method for registering with ghost diff --git a/core/frontend/helpers/paginate.js b/core/frontend/helpers/paginate.js index 070d966be1e6..5ff4130df7e3 100644 --- a/core/frontend/helpers/paginate.js +++ b/core/frontend/helpers/paginate.js @@ -20,7 +20,7 @@ PaginationHelper = function (paginationTemplate) { PaginationHelper.prototype.compileTemplate = function (templatePath) { var self = this; - + // Allow people to overwrite the paginationTemplatePath templatePath = templatePath || this.paginationTemplatePath; return nodefn.call(fs.readFile, templatePath).then(function (paginationContents) { @@ -29,12 +29,10 @@ PaginationHelper.prototype.compileTemplate = function (templatePath) { }); }; -PaginationHelper.prototype.renderPagination = function (context, options) { - var output = this.paginationTemplateFunc(context); - return output; +PaginationHelper.prototype.renderPagination = function (context) { + return new handlebars.SafeString(this.paginationTemplateFunc(context)); }; - PaginationHelper.registerWithGhost = function (ghost) { var templatePath = path.join(ghost.paths().frontendViews, 'pagination.hbs'), paginationHelper = new PaginationHelper(templatePath); diff --git a/core/test/ghost/frontend_helpers_ghostNav_spec.js b/core/test/ghost/frontend_helpers_navigation_spec.js similarity index 97% rename from core/test/ghost/frontend_helpers_ghostNav_spec.js rename to core/test/ghost/frontend_helpers_navigation_spec.js index a044b0b78d0f..84aafad41e91 100644 --- a/core/test/ghost/frontend_helpers_ghostNav_spec.js +++ b/core/test/ghost/frontend_helpers_navigation_spec.js @@ -37,7 +37,7 @@ describe('Navigation Helper', function () { // Returns a string returned from navTemplateFunc should.exist(rendered); - rendered.should.equal("rendered 2"); + rendered.string.should.equal("rendered 2"); templateSpy.calledWith({ links: fakeNavItems }).should.equal(true); }); From 0dd0d2067815e49eb599c8211fc08fc304b62b5c Mon Sep 17 00:00:00 2001 From: ErisDS Date: Wed, 10 Jul 2013 16:54:29 +0100 Subject: [PATCH 2/3] Nav helper bug - home page always marked as current - 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. --- core/frontend/filters/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/frontend/filters/index.js b/core/frontend/filters/index.js index 32d022c65f5c..3252d78d8013 100644 --- a/core/frontend/filters/index.js +++ b/core/frontend/filters/index.js @@ -6,8 +6,12 @@ coreFilters = function (ghost) { ghost.registerFilter('ghostNavItems', defaultCoreFilterPriority, function (args) { var selectedItem; - // Set the nav items based on the config - args.navItems = ghost.config().nav; + // we want to clone the config so the config remains unchanged + // we will need to make this recursive if we start supporting + // hierarchical menus + args.navItems = _.map(ghost.config().nav, function (value) { + return Object.create(value); + }); // Mark the current selected Item selectedItem = _.find(args.navItems, function (item) { From 6f8752aa224e5d030c553ef0891507630eba70f9 Mon Sep 17 00:00:00 2001 From: ErisDS Date: Wed, 10 Jul 2013 23:45:13 +0100 Subject: [PATCH 3/3] HTML helpers refactor - issue #246 items 2 and 5. - 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 --- core/frontend/helpers/index.js | 42 ++++++++++-- core/frontend/helpers/navigation.js | 46 ------------- core/frontend/helpers/paginate.js | 45 ------------- core/ghost.js | 17 +++++ core/test/ghost/fixtures/test.hbs | 1 + .../test/ghost/frontend_helpers_index_spec.js | 55 ++++++++++++++++ .../ghost/frontend_helpers_navigation_spec.js | 65 ------------------- core/test/ghost/ghost_spec.js | 60 ++++++++++++++--- 8 files changed, 162 insertions(+), 169 deletions(-) delete mode 100644 core/frontend/helpers/navigation.js delete mode 100644 core/frontend/helpers/paginate.js create mode 100644 core/test/ghost/fixtures/test.hbs create mode 100644 core/test/ghost/frontend_helpers_index_spec.js delete mode 100644 core/test/ghost/frontend_helpers_navigation_spec.js diff --git a/core/frontend/helpers/index.js b/core/frontend/helpers/index.js index 5213f0fe48e2..b484dcbec0bc 100644 --- a/core/frontend/helpers/index.js +++ b/core/frontend/helpers/index.js @@ -1,12 +1,14 @@ var _ = require('underscore'), moment = require('moment'), when = require('when'), - pagination = require('./paginate'), - navHelper = require('./navigation'), hbs = require('express-hbs'), + errors = require('../../shared/errorHandling'), coreHelpers; coreHelpers = function (ghost) { + var navHelper, + paginationHelper; + /** * [ description] * @todo ghost core helpers + a way for themes to register them @@ -119,10 +121,40 @@ coreHelpers = function (ghost) { } return ret; }); - // Just one async helper for now, but could be more in the future + + // ## Template driven helpers + // Template driven helpers require that their template is loaded before they can be registered. + + // ###Nav Helper + // `{{nav}}` + // Outputs a navigation menu built from items in config.js + navHelper = ghost.loadTemplate('nav').then(function (templateFn) { + ghost.registerThemeHelper('nav', function (options) { + if (!_.isObject(this.navItems) || _.isFunction(this.navItems)) { + errors.logAndThrowError('navItems data is not an object or is a function'); + return; + } + return new hbs.handlebars.SafeString(templateFn({links: this.navItems})); + }); + }); + + // ### Pagination Helper + // `{{paginate}}` + // Outputs previous and next buttons, along with info about the current page + paginationHelper = ghost.loadTemplate('pagination').then(function (templateFn) { + ghost.registerThemeHelper('paginate', function (options) { + if (!_.isObject(this.pagination) || _.isFunction(this.pagination)) { + errors.logAndThrowError('pagination data is not an object or is a function'); + return; + } + return new hbs.handlebars.SafeString(templateFn(this.pagination)); + }); + }); + + // Return once the template-driven helpers have loaded return when.join( - navHelper.registerWithGhost(ghost), - pagination.registerWithGhost(ghost) + navHelper, + paginationHelper ); }; diff --git a/core/frontend/helpers/navigation.js b/core/frontend/helpers/navigation.js deleted file mode 100644 index 0ed7a4a52698..000000000000 --- a/core/frontend/helpers/navigation.js +++ /dev/null @@ -1,46 +0,0 @@ -var fs = require('fs'), - path = require('path'), - _ = require('underscore'), - handlebars = require('express-hbs').handlebars, - nodefn = require('when/node/function'), - - NavHelper; - -NavHelper = function (navTemplate) { - // Bind the context for our methods. - _.bindAll(this, 'compileTemplate', 'renderNavItems'); - - if (_.isFunction(navTemplate)) { - this.navTemplateFunc = navTemplate; - } else { - this.navTemplatePath = navTemplate; - } -}; - -NavHelper.prototype.compileTemplate = function (templatePath) { - var self = this; - - // Allow people to overwrite the navTemplatePath - templatePath = templatePath || this.navTemplatePath; - - return nodefn.call(fs.readFile, templatePath).then(function (navTemplateContents) { - // TODO: Can handlebars compile async? - self.navTemplateFunc = handlebars.compile(navTemplateContents.toString()); - }); -}; - -NavHelper.prototype.renderNavItems = function (navItems) { - return new handlebars.SafeString(this.navTemplateFunc({links: navItems})); -}; - -// A static helper method for registering with ghost -NavHelper.registerWithGhost = function (ghost) { - var templatePath = path.join(ghost.paths().frontendViews, 'nav.hbs'), - ghostNavHelper = new NavHelper(templatePath); - - return ghostNavHelper.compileTemplate().then(function () { - ghost.registerThemeHelper("nav", ghostNavHelper.renderNavItems); - }); -}; - -module.exports = NavHelper; \ No newline at end of file diff --git a/core/frontend/helpers/paginate.js b/core/frontend/helpers/paginate.js deleted file mode 100644 index 5ff4130df7e3..000000000000 --- a/core/frontend/helpers/paginate.js +++ /dev/null @@ -1,45 +0,0 @@ -var fs = require('fs'), - path = require('path'), - _ = require('underscore'), - handlebars = require('express-hbs').handlebars, - nodefn = require('when/node/function'), - - PaginationHelper; - -PaginationHelper = function (paginationTemplate) { - // Bind the context for our methods. - _.bindAll(this, 'compileTemplate', 'renderPagination'); - - if (_.isFunction(paginationTemplate)) { - this.paginationTemplateFunc = paginationTemplate; - } else { - this.paginationTemplatePath = paginationTemplate; - } -}; - -PaginationHelper.prototype.compileTemplate = function (templatePath) { - var self = this; - - // Allow people to overwrite the paginationTemplatePath - templatePath = templatePath || this.paginationTemplatePath; - - return nodefn.call(fs.readFile, templatePath).then(function (paginationContents) { - // TODO: Can handlebars compile async? - self.paginationTemplateFunc = handlebars.compile(paginationContents.toString()); - }); -}; - -PaginationHelper.prototype.renderPagination = function (context) { - return new handlebars.SafeString(this.paginationTemplateFunc(context)); -}; - -PaginationHelper.registerWithGhost = function (ghost) { - var templatePath = path.join(ghost.paths().frontendViews, 'pagination.hbs'), - paginationHelper = new PaginationHelper(templatePath); - - return paginationHelper.compileTemplate().then(function () { - ghost.registerThemeHelper("paginate", paginationHelper.renderPagination); - }); -}; - -module.exports = PaginationHelper; \ No newline at end of file diff --git a/core/ghost.js b/core/ghost.js index 59c999e1d783..0d0e0b4bf800 100644 --- a/core/ghost.js +++ b/core/ghost.js @@ -6,8 +6,10 @@ var config = require('./../config'), when = require('when'), express = require('express'), errors = require('../core/shared/errorHandling'), + fs = require('fs'), path = require('path'), hbs = require('express-hbs'), + nodefn = require('when/node/function'), _ = require('underscore'), Polyglot = require('node-polyglot'), @@ -167,6 +169,21 @@ Ghost.prototype.updateSettingsCache = function (settings) { } }; +// ## Template utils + +Ghost.prototype.compileTemplate = function (templatePath) { + return nodefn.call(fs.readFile, templatePath).then(function (templateContents) { + return hbs.handlebars.compile(templateContents.toString()); + }, errors.logAndThrowError); +}; + +Ghost.prototype.loadTemplate = function (name) { + // TODO: allow themes to override these templates + var templatePath = path.join(this.paths().frontendViews, name + '.hbs'); + + return this.compileTemplate(templatePath); +}; + /** * @param {string} name * @param {Function} fn diff --git a/core/test/ghost/fixtures/test.hbs b/core/test/ghost/fixtures/test.hbs new file mode 100644 index 000000000000..b8264bd9f355 --- /dev/null +++ b/core/test/ghost/fixtures/test.hbs @@ -0,0 +1 @@ +

HelloWorld

\ No newline at end of file diff --git a/core/test/ghost/frontend_helpers_index_spec.js b/core/test/ghost/frontend_helpers_index_spec.js new file mode 100644 index 000000000000..6aed2f581303 --- /dev/null +++ b/core/test/ghost/frontend_helpers_index_spec.js @@ -0,0 +1,55 @@ +/*globals describe, beforeEach, it*/ +var should = require('should'), + sinon = require('sinon'), + when = require('when'), + _ = require('underscore'), + handlebars = require('express-hbs').handlebars, + path = require('path'), + helpers = require('../../frontend/helpers'), + Ghost = require('../../ghost'); + +describe('Core Helpers', function () { + + var ghost; + + beforeEach(function () { + ghost = new Ghost(); + }); + + + describe('Navigation Helper', function () { + + it('can render nav items', function (done) { + var templateSpy = sinon.spy(function (data) { return "rendered " + data.links.length; }), + compileSpy = sinon.stub(ghost, 'compileTemplate').returns(when.resolve(templateSpy)), + fakeNavItems = [{ + title: 'test1', + url: '/test1' + }, { + title: 'test2', + url: '/test2' + }], + rendered; + + helpers.loadCoreHelpers(ghost).then(function () { + + should.exist(handlebars.helpers.nav); + + rendered = handlebars.helpers.nav.call({navItems: fakeNavItems}); + + // Returns a string returned from navTemplateFunc + should.exist(rendered); + rendered.string.should.equal("rendered 2"); + + compileSpy.called.should.equal(true); + templateSpy.called.should.equal(true); + templateSpy.calledWith({ links: fakeNavItems }).should.equal(true); + + + compileSpy.restore(); + + done(); + }, done); + }); + }); +}); \ No newline at end of file diff --git a/core/test/ghost/frontend_helpers_navigation_spec.js b/core/test/ghost/frontend_helpers_navigation_spec.js deleted file mode 100644 index 84aafad41e91..000000000000 --- a/core/test/ghost/frontend_helpers_navigation_spec.js +++ /dev/null @@ -1,65 +0,0 @@ -/*globals describe, beforeEach, it*/ -var should = require('should'), - sinon = require('sinon'), - _ = require('underscore'), - path = require('path'), - NavHelper = require('../../frontend/helpers/navigation'); - -describe('Navigation Helper', function () { - var navTemplatePath = path.join(process.cwd(), 'core/frontend/views/nav.hbs'); - - should.exist(NavHelper, "Navigation helper exists"); - - it('can compile the nav template', function (done) { - var helper = new NavHelper(navTemplatePath); - - helper.compileTemplate().then(function () { - should.exist(helper.navTemplateFunc); - _.isFunction(helper.navTemplateFunc).should.equal(true); - - done(); - }, done); - }); - - it('can render nav items', function () { - var helper = new NavHelper(function (data) { return "rendered " + data.links.length; }), - templateSpy = sinon.spy(helper, 'navTemplateFunc'), - fakeNavItems = [{ - title: 'test1', - url: '/test1' - }, { - title: 'test2', - url: '/test2' - }], - rendered; - - rendered = helper.renderNavItems(fakeNavItems); - - // Returns a string returned from navTemplateFunc - should.exist(rendered); - rendered.string.should.equal("rendered 2"); - - templateSpy.calledWith({ links: fakeNavItems }).should.equal(true); - }); - - it('can register with ghost', function (done) { - var fakeGhost = { - paths: function () { - return { - frontendViews: path.join(process.cwd(), 'core/frontend/views/') - }; - }, - - registerThemeHelper: function () { - return; - } - }, - registerStub = sinon.stub(fakeGhost, 'registerThemeHelper'); - - NavHelper.registerWithGhost(fakeGhost).then(function () { - registerStub.called.should.equal(true); - - done(); - }, done); - }); -}); \ No newline at end of file diff --git a/core/test/ghost/ghost_spec.js b/core/test/ghost/ghost_spec.js index 109da829452b..ba35e58110eb 100644 --- a/core/test/ghost/ghost_spec.js +++ b/core/test/ghost/ghost_spec.js @@ -2,9 +2,17 @@ var should = require('should'), when = require('when'), sinon = require('sinon'), + path = require('path'), + _ = require('underscore'), Ghost = require('../../ghost'); describe("Ghost API", function () { + var testTemplatePath = 'core/test/ghost/fixtures/', + ghost; + + beforeEach(function () { + ghost = new Ghost(); + }); it("is a singleton", function () { var logStub = sinon.stub(console, "log"), @@ -16,8 +24,7 @@ describe("Ghost API", function () { }); it("uses init() to initialize", function (done) { - var ghost = new Ghost(), - fakeDataProvider = { + var fakeDataProvider = { init: function () { return when.resolve(); } @@ -43,8 +50,7 @@ describe("Ghost API", function () { }); it("can register filters with specific priority", function () { - var ghost = new Ghost(), - filterName = 'test', + var filterName = 'test', filterPriority = 9, testFilterHandler = sinon.spy(); @@ -57,8 +63,7 @@ describe("Ghost API", function () { }); it("can register filters with default priority", function () { - var ghost = new Ghost(), - filterName = 'test', + var filterName = 'test', defaultPriority = 5, testFilterHandler = sinon.spy(); @@ -71,8 +76,7 @@ describe("Ghost API", function () { }); it("executes filters in priority order", function (done) { - var ghost = new Ghost(), - filterName = 'testpriority', + var filterName = 'testpriority', testFilterHandler1 = sinon.spy(), testFilterHandler2 = sinon.spy(), testFilterHandler3 = sinon.spy(); @@ -91,4 +95,44 @@ describe("Ghost API", function () { done(); }); }); + + it("can compile a template", function (done) { + var template = path.join(process.cwd(), testTemplatePath, 'test.hbs'); + + should.exist(ghost.compileTemplate, 'Template Compiler exists'); + + ghost.compileTemplate(template).then(function (templateFn) { + should.exist(templateFn); + _.isFunction(templateFn).should.equal(true); + + templateFn().should.equal('

HelloWorld

'); + done(); + }, done); + }); + + it("loads templates for helpers", function (done) { + var compileSpy = sinon.spy(ghost, 'compileTemplate'); + + should.exist(ghost.loadTemplate, 'load template function exists'); + + // In order for the test to work, need to replace the path to the template + ghost.paths = sinon.stub().returns({ + frontendViews: path.join(process.cwd(), testTemplatePath) + }); + + ghost.loadTemplate('test').then(function (templateFn) { + // test that compileTemplate was called with the expected path + compileSpy.calledOnce.should.equal(true); + compileSpy.calledWith(path.join(process.cwd(), testTemplatePath, 'test.hbs')).should.equal(true); + + should.exist(templateFn); + _.isFunction(templateFn).should.equal(true); + + templateFn().should.equal('

HelloWorld

'); + + compileSpy.restore(); + + done(); + }, done); + }); }); \ No newline at end of file