From b48fdaf1be371ace3021927898a6bdaed66fd69d Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 29 Jul 2019 18:54:15 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8Added=20{{link=5Fclass}}=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - moved dynamic class logic out of {{link}} helper into shared utils - both {{link}} and {{link_class}} use these utils --- core/frontend/helpers/index.js | 2 + core/frontend/helpers/link.js | 87 ++----- core/frontend/helpers/link_class.js | 29 +++ core/frontend/helpers/utils.js | 55 +++++ core/server/translations/en.json | 6 + core/test/unit/helpers/index_spec.js | 2 +- core/test/unit/helpers/link_class_spec.js | 276 ++++++++++++++++++++++ core/test/unit/helpers/link_spec.js | 29 +-- 8 files changed, 404 insertions(+), 82 deletions(-) create mode 100644 core/frontend/helpers/link_class.js create mode 100644 core/test/unit/helpers/link_class_spec.js diff --git a/core/frontend/helpers/index.js b/core/frontend/helpers/index.js index a11c9a49c2..f0e5b261c8 100644 --- a/core/frontend/helpers/index.js +++ b/core/frontend/helpers/index.js @@ -23,6 +23,7 @@ coreHelpers.is = require('./is'); coreHelpers.has = require('./has'); coreHelpers.lang = require('./lang'); coreHelpers.link = require('./link'); +coreHelpers.link_class = require('./link_class'); coreHelpers.meta_description = require('./meta_description'); coreHelpers.meta_title = require('./meta_title'); coreHelpers.navigation = require('./navigation'); @@ -57,6 +58,7 @@ registerAllCoreHelpers = function registerAllCoreHelpers() { registerThemeHelper('img_url', coreHelpers.img_url); registerThemeHelper('lang', coreHelpers.lang); registerThemeHelper('link', coreHelpers.link); + registerThemeHelper('link_class', coreHelpers.link_class); registerThemeHelper('meta_description', coreHelpers.meta_description); registerThemeHelper('meta_title', coreHelpers.meta_title); registerThemeHelper('navigation', coreHelpers.navigation); diff --git a/core/frontend/helpers/link.js b/core/frontend/helpers/link.js index af1c171d4a..33c2fe7626 100644 --- a/core/frontend/helpers/link.js +++ b/core/frontend/helpers/link.js @@ -1,51 +1,9 @@ // # link helper const _ = require('lodash'); -const {config, SafeString} = require('./proxy'); +const {config, SafeString, errors, i18n} = require('./proxy'); +const {buildLinkClasses} = require('./utils'); -const managedAttributes = ['href', 'class', 'activeClass', 'parentActiveClass', 'tagName', 'nohref']; - -function _getHref(hash) { - let href = hash.href || '/'; - return href.string ? href.string : href; -} - -function _clean(url) { - // Strips anchors and leading and trailing slashes - return url.replace(/#.*?$/, '').replace(/^\/|\/$/g, ''); -} - -// strips trailing slashes and compares urls -function _urlMatch(href, location) { - if (!location) { - return false; - } - - const strippedHref = _clean(href); - const strippedLocation = _clean(location); - - return strippedHref === strippedLocation; -} - -// We want to check if the first part of the current url is a match for href -function _parentMatch(href, location) { - if (!location) { - return false; - } - - let parent = false; - let locParts = _clean(location).split('/'); - let hrefParts = _clean(href).split('/'); - - if (locParts.length <= hrefParts.length) { - return false; - } - - for (let i = 0; i < hrefParts.length; i += 1) { - parent = hrefParts[i] === locParts[i]; - } - - return parent; -} +const managedAttributes = ['href', 'class', 'activeClass', 'parentActiveClass']; function _formatAttrs(attributes) { let attributeString = ''; @@ -64,13 +22,23 @@ module.exports = function link(options) { options.hash = options.hash || {}; options.data = options.data || {}; - let href = _getHref(options.hash); - let location = options.data.root.relativeUrl; - let tagName = options.hash.tagName || 'a'; - let activeClass = _.has(options.hash, 'activeClass') ? options.hash.activeClass : 'nav-current'; - let parentActiveClass = _.has(options.hash, 'parentActiveClass') ? options.hash.parentActiveClass : `${activeClass || 'nav-current'}-parent`; - let classes = options.hash.class ? options.hash.class.toString().split(' ') : []; - let noHref = _.has(options.hash, 'nohref') ? options.hash.nohref : false; + // If there is no href provided, this is theme dev error, so we throw an error to make this clear. + if (!_.has(options.hash, 'href')) { + throw new errors.IncorrectUsageError({ + message: i18n.t('warnings.helpers.link.hrefIsRequired') + }); + } + // If the href attribute is empty, this is probably a dynamic data problem, hard for theme devs to track down + // E.g. {{#link for=slug}}{{/link}} in a context where slug returns an empty string + // Error's here aren't useful (same as with empty get helper filters) so we fallback gracefully + if (!options.hash.href) { + options.hash.href = ''; + } + + let href = options.hash.href.string || options.hash.href; + + // Calculate dynamic properties + let classes = buildLinkClasses(config.get('url'), href, options); // Remove all the attributes we don't want to do a one-to-one mapping of managedAttributes.forEach((attr) => { @@ -80,20 +48,13 @@ module.exports = function link(options) { // Setup our one-to-one mapping of attributes; let attributes = options.hash; - // Calculate dynamic properties - let relativeHref = href.replace(config.get('url'), ''); - if (_urlMatch(relativeHref, location) && activeClass) { - classes.push(activeClass); - } else if (_parentMatch(relativeHref, location) && parentActiveClass) { - classes.push(parentActiveClass); - } - // Prepare output let classString = classes.length > 0 ? `class="${classes.join(' ')}"` : ''; - let hrefString = !noHref ? `href="${href}"` : ''; + let hrefString = `href="${href}"`; let attributeString = _.size(attributes) > 0 ? _formatAttrs(attributes) : ''; - let openingTag = `<${tagName} ${classString} ${hrefString} ${attributeString}>`; - let closingTag = ``; + let openingTag = ``; + let closingTag = ``; + // Clean up any extra spaces openingTag = openingTag.replace(/\s{2,}/g, ' ').replace(/\s>/, '>'); diff --git a/core/frontend/helpers/link_class.js b/core/frontend/helpers/link_class.js new file mode 100644 index 0000000000..bbe8fb7c2f --- /dev/null +++ b/core/frontend/helpers/link_class.js @@ -0,0 +1,29 @@ +// # link_class helper +const _ = require('lodash'); +const {config, SafeString, errors, i18n} = require('./proxy'); +const {buildLinkClasses} = require('./utils'); + +module.exports = function link_class(options) { // eslint-disable-line camelcase + options = options || {}; + options.hash = options.hash || {}; + options.data = options.data || {}; + + // If there is no for provided, this is theme dev error, so we throw an error to make this clear. + if (!_.has(options.hash, 'for')) { + throw new errors.IncorrectUsageError({ + message: i18n.t('warnings.helpers.link_class.forIsRequired') + }); + } + + // If the for attribute is present but empty, this is probably a dynamic data problem, hard for theme devs to track down + // E.g. {{link_class for=slug}} in a context where slug returns an empty string + // Error's here aren't useful (same as with empty get helper filters) so we fallback gracefully + if (!options.hash.for) { + options.hash.for = ''; + } + + let href = options.hash.for.string || options.hash.for; + let classes = buildLinkClasses(config.get('url'), href, options); + + return new SafeString(classes.join(' ')); +}; diff --git a/core/frontend/helpers/utils.js b/core/frontend/helpers/utils.js index e2b0201805..005fb17cae 100644 --- a/core/frontend/helpers/utils.js +++ b/core/frontend/helpers/utils.js @@ -11,3 +11,58 @@ module.exports.findKey = function findKey(key /* ...objects... */) { return result; }, null); }; + +function _urlClean(url) { + // Strips anchors and leading and trailing slashes + return url.replace(/#.*?$/, '').replace(/^\/|\/$/g, ''); +} + +// strips trailing slashes and compares urls +function _urlMatch(href, location) { + if (!location) { + return false; + } + + const strippedHref = _urlClean(href); + const strippedLocation = _urlClean(location); + + return strippedHref === strippedLocation; +} + +// We want to check if the first part of the current url is a match for href +function _urlParentMatch(href, location) { + if (!location) { + return false; + } + + let parent = false; + let locParts = _urlClean(location).split('/'); + let hrefParts = _urlClean(href).split('/'); + + if (locParts.length <= hrefParts.length) { + return false; + } + + for (let i = 0; i < hrefParts.length; i += 1) { + parent = hrefParts[i] === locParts[i]; + } + + return parent; +} + +module.exports.buildLinkClasses = function buildLinkClasses(siteUrl, href, options) { + let relativeHref = href.replace(siteUrl, ''); + let location = options.data.root.relativeUrl; + let classes = options.hash.class ? options.hash.class.toString().split(' ') : []; + let activeClass = _.has(options.hash, 'activeClass') ? options.hash.activeClass : 'nav-current'; + let parentActiveClass = _.has(options.hash, 'parentActiveClass') ? options.hash.parentActiveClass : `${activeClass || 'nav-current'}-parent`; + + // Calculate dynamic properties + if (_urlMatch(relativeHref, location) && activeClass) { + classes.push(activeClass); + } else if (_urlParentMatch(relativeHref, location) && parentActiveClass) { + classes.push(parentActiveClass); + } + + return classes; +}; diff --git a/core/server/translations/en.json b/core/server/translations/en.json index b0736f9a57..771b49f009 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -596,6 +596,12 @@ "is": { "invalidAttribute": "Invalid or no attribute given to is helper" }, + "link": { + "hrefIsRequired": "The \\{\\{#link\\}\\}\\{\\{/link\\}\\} helper requires an href=\"\" attribute." + }, + "link_class": { + "forIsRequired": "The \\{\\{link_class\\}\\} helper requires a for=\"\" attribute." + }, "navigation": { "invalidData": "navigation data is not an object or is a function", "valuesMustBeDefined": "All values must be defined for label, url and current", diff --git a/core/test/unit/helpers/index_spec.js b/core/test/unit/helpers/index_spec.js index 37a5e867f9..56d277cdc5 100644 --- a/core/test/unit/helpers/index_spec.js +++ b/core/test/unit/helpers/index_spec.js @@ -9,7 +9,7 @@ describe('Helpers', function () { var hbsHelpers = ['each', 'if', 'unless', 'with', 'helperMissing', 'blockHelperMissing', 'log', 'lookup', 'block', 'contentFor'], ghostHelpers = [ 'asset', 'author', 'authors', 'body_class', 'concat', 'content', 'date', 'encode', 'excerpt', 'facebook_url', 'foreach', 'get', - 'ghost_foot', 'ghost_head', 'has', 'img_url', 'is', 'lang', 'link', 'meta_description', 'meta_title', 'navigation', + 'ghost_foot', 'ghost_head', 'has', 'img_url', 'is', 'lang', 'link', 'link_class', 'meta_description', 'meta_title', 'navigation', 'next_post', 'page_url', 'pagination', 'plural', 'post_class', 'prev_post', 'reading_time', 't', 'tags', 'title', 'twitter_url', 'url' ], diff --git a/core/test/unit/helpers/link_class_spec.js b/core/test/unit/helpers/link_class_spec.js new file mode 100644 index 0000000000..5f832ccd53 --- /dev/null +++ b/core/test/unit/helpers/link_class_spec.js @@ -0,0 +1,276 @@ +const should = require('should'); +const helpers = require.main.require('core/frontend/helpers'); +const handlebars = require.main.require('core/frontend/services/themes/engine').handlebars; + +const configUtils = require('../../utils/configUtils'); + +let defaultGlobals; + +function compile(templateString) { + const template = handlebars.compile(templateString); + template.with = (locals = {}, globals) => { + globals = globals || defaultGlobals; + + return template(locals, globals); + }; + + return template; +} + +describe('{{link_class}} helper', function () { + before(function () { + handlebars.registerHelper('link_class', helpers.link_class); + handlebars.registerHelper('concat', helpers.concat); + configUtils.config.set('url', 'https://siteurl.com'); + defaultGlobals = { + data: { + site: { + url: configUtils.config.get('url') + } + } + }; + }); + + after(function () { + configUtils.restore(); + }); + + it('throws an error for missing for=""', function () { + (function compileWith() { + compile('{{link_class}}') + .with({}); + }).should.throw(); + }); + + it('silently accepts an empty for...', function () { + compile('{{link_class for=tag.slug}}') + .with({tag: null}) + .should.eql(''); + }); + + describe('activeClass', function () { + it('gets applied correctly', function () { + // Test matching relative URL + compile('{{link_class for="/about/"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-current'); + + // Test non-matching relative URL + compile('{{link_class for="/about/"}}') + .with({relativeUrl: '/'}) + .should.eql(''); + }); + + it('ignores anchors', function () { + // Anchor in href + compile('{{link_class for="/about/#me"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-current'); + + // Anchor in relative URL + compile('{{link_class for="/about/"}}') + .with({relativeUrl: '/about/#me'}) + .should.eql('nav-current'); + }); + + it('handles missing trailing slashes', function () { + compile('{{link_class for="/about"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-current'); + }); + + it('handles absolute URLs', function () { + // Correct URL gets class + compile('{{link_class for="https://siteurl.com/about/"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-current'); + + // Incorrect URL doesn't get class + compile('{{link_class for="https://othersite.com/about/"}}') + .with({relativeUrl: '/about/'}) + .should.eql(''); + }); + + it('handles absolute URL with missing trailing slash', function () { + compile('{{link_class for="https://siteurl.com/about"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-current'); + }); + + it('activeClass can be customised', function () { + compile('{{link_class for="/about/" activeClass="nav-active"}}') + .with({relativeUrl: '/about/'}) + .should.eql('nav-active'); + + compile('{{link_class for="/about/" activeClass=slug}}') + .with({relativeUrl: '/about/', slug: 'fred'}) + .should.eql('fred'); + + compile('{{link_class for="/about/" activeClass=(concat slug "active" separator="-")}}') + .with({relativeUrl: '/about/', slug: 'fred'}) + .should.eql('fred-active'); + }); + + it('activeClass and other classes work together', function () { + // Single extra class + compile('{{link_class for="/about/" class="about"}}') + .with({relativeUrl: '/about/'}) + .should.eql('about nav-current'); + + // Multiple extra classes + compile('{{link_class for="/about/" class="about my-link"}}') + .with({relativeUrl: '/about/'}) + .should.eql('about my-link nav-current'); + }); + + it('can disable activeClass with falsey values', function () { + // Empty string + compile('{{link_class for="/about/" activeClass=""}}') + .with({relativeUrl: '/about/'}) + .should.eql(''); + + // false + compile('{{link_class for="/about/" activeClass=false}}') + .with({relativeUrl: '/about/'}) + .should.eql(''); + }); + }); + + describe('parentActiveClass', function () { + it('gets applied correctly', function () { + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('ignores anchors', function () { + // Anchor in href + + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + // Anchor in relative URL + + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/#me'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/#me'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('handles missing trailing slashes', function () { + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('handles absolute URLs', function () { + // Correct URL gets class + + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + // Incorrect URL doesn't get class + + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('handles absolute URLs with missing trailing slashes', function () { + // Parent and child links with PARENT as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/'}) + .should.eql('
  • parent
  • child
    • '); + + // Parent and child links with CHILD as relative URL + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('parentActiveClass can be customised', function () { + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/', slug: 'fred'}) + .should.eql('
  • parent
  • child
    • '); + + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/', slug: 'fred'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('customising activeClass also customises parentActiveClass', function () { + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + + it('can disable parentActiveClass with falsey values', function () { + compile('{{link_class for="/about/" parentActiveClass=""}}') + .with({relativeUrl: '/about/team/'}) + .should.eql(''); + + compile('{{link_class for="/about/" parentActiveClass=false}}') + .with({relativeUrl: '/about/team/'}) + .should.eql(''); + }); + + it('disabling activeClass does not affect parentActiveClass', function () { + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + + compile('
  • parent
  • child
    • ') + .with({relativeUrl: '/about/team/'}) + .should.eql('
  • parent
  • child
    • '); + }); + }); +}); diff --git a/core/test/unit/helpers/link_spec.js b/core/test/unit/helpers/link_spec.js index 757652e80c..ed1f7bc3f5 100644 --- a/core/test/unit/helpers/link_spec.js +++ b/core/test/unit/helpers/link_spec.js @@ -37,10 +37,17 @@ describe('{{link}} helper', function () { }); describe('basic behaviour: simple links without context', function () { - it('basic tag with default url', function () { - compile('{{#link}}text{{/link}}') - .with({}) - .should.eql('text'); + it('throws an error for missing href=""', function () { + (function compileWith() { + compile('{{#link}}text{{/link}}') + .with({}); + }).should.throw(); + }); + + it('silently accepts an empty href...', function () { + compile('{{#link href=tag.slug}}text{{/link}}') + .with({tag: null}) + .should.eql('text'); }); it(' tag with a specific URL', function () { @@ -338,19 +345,5 @@ describe('{{link}} helper', function () { .should.eql('parentchild'); }); }); - - describe('custom tag', function () { - it('can change tag', function () { - compile('{{#link href="/about/" class="my-class" tagName="li"}}text{{/link}}') - .with({relativeUrl: '/about/'}) - .should.eql('
  • text
    • '); - }); - - it('can change tag and disable href', function () { - compile('{{#link href="/about/" class="my-class" tagName="li" nohref=true}}text{{/link}}') - .with({relativeUrl: '/about/'}) - .should.eql('
  • text
    • '); - }); - }); }); });