Skip to content

Commit

Permalink
pre work for replacing IconComponent usages with iconPartial (#870)
Browse files Browse the repository at this point in the history
This is the work needed before replacing IconComponent usages with iconPartial.
Since there are many icon usages, it wouild be hard to see these changes if they were
combined with the replacements.

- register the relativePathHandler for SDK components so it can be used at runtime
- fix bug in relativePathHandler where if the url is undefined, it will return something
  like "./undefined" instead of the value undefined. In previous usages of the helper,
  url was guarded from being null/undefined, but this won't be the case for iconPartial
- add percy snapshots for a card with custom cta icons
- fix test-sites gitignore negations not working properly
- point the theme's test-site to the feature/icon-partial-i18n so we get the icon-partial change, and also i18n-ed assets so the spanish snapshots work
- also collect code coverage on the hbshelpers folder

J=SLAP-1297
TEST=manual,auto

test that custom icons still work, both for section titles on universal
and inside cards

unit tests for relativePathHandler
  • Loading branch information
oshi97 committed Jul 1, 2021
1 parent 00034ef commit 2acdda1
Show file tree
Hide file tree
Showing 16 changed files with 183 additions and 20 deletions.
2 changes: 1 addition & 1 deletion commands/helpers/utils/argumentmetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ Object.freeze(ArgumentType);
class ArgumentMetadata {
constructor(type, description, isRequired, defaultValue, itemType) {
this._type = type;
this._description = description;
this._isRequired = isRequired;
this._defaultValue = defaultValue;
this._description = description;
this._itemType = itemType;
}

Expand Down
5 changes: 3 additions & 2 deletions hbshelpers/relativePathHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ const isNonRelativeUrl = require('./isNonRelativeUrl');
* If the url is not relative, return it. If it is relative,
* append relativePath to it.
*
* @param {string} url
* @param {import('handlebars').HelperOptions} options
* @param {string} options.hash.relativePath
* @param {string} options.hash.url
* @returns {string}
*/
module.exports = function relativePathHandler(options) {
const { relativePath, url } = options.hash || {};
if (isNonRelativeUrl(url) || !relativePath) {
if (isNonRelativeUrl(url) || !relativePath || !url) {
return url;
}
return relativePath + '/' + url;
Expand Down
4 changes: 3 additions & 1 deletion hbshelpers/sdkAssetUrl.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const RELEASE_BRANCH_REGEX = /^release\/v[0-9.]+$/;
const HOTFIX_BRANCH_REGEX = /^hotfix\/v[0-9.]+$/;
const I18N_FEATURE_BRANCH_REGEX = /^feature\/.+-i18n$/;
const SEM_VER_REGEX = /^[1-9]+$|^[1-9]+\.[0-9]+$|^[1-9]+\.[0-9]+\.[0-9]+$/;

/**
Expand All @@ -25,8 +26,9 @@ module.exports = function sdkAssetUrl(branch, locale, assetName) {

const isPreReleaseBranch =
RELEASE_BRANCH_REGEX.test(branch) || HOTFIX_BRANCH_REGEX.test(branch);
const isI18nFeatureBranch = I18N_FEATURE_BRANCH_REGEX.test(branch);
const isLocalizationSupported =
(isReleasedBranch || isPreReleaseBranch) &&
(isReleasedBranch || isPreReleaseBranch || isI18nFeatureBranch) &&
!(locale.startsWith('en') || assetName === 'answers.css') ;

const parsedAssetName = isLocalizationSupported ?
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
},
"jest": {
"collectCoverageFrom": [
"static/**/*.js"
"static/**/*.js",
"hbshelpers/*.js"
],
"verbose": true,
"moduleFileExtensions": [
Expand Down
22 changes: 20 additions & 2 deletions script/core.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,32 @@
* Determine whether a URL is absolute or not.
* Common examples: "mailto:slapshot@gmail.com", "//yext.com", "https://yext.com"
*/
ANSWERS.registerHelper('isNonRelativeUrl', function(str) {
function isNonRelativeUrl(str) {
const absoluteURLRegex = /^(\/|[a-zA-Z]+:)/;
return str && str.match(absoluteURLRegex);
});
}
ANSWERS.registerHelper('isNonRelativeUrl', isNonRelativeUrl);
ANSWERS.registerHelper('close-card-svg', () => {
return ANSWERS.renderer.SafeString({{{stringifyPartial (read 'static/assets/images/close-card') }}});
});
/**
* If the url is not relative, return it. If it is relative,
* append relativePath to it.
*
* @param {import('handlebars').HelperOptions} options
* @param {string} options.hash.relativePath
* @param {string} options.hash.url
* @returns {string}
*/
ANSWERS.registerHelper('relativePathHandler', function relativePathHandler(options) {
const { relativePath, url } = options.hash || {};
if (isNonRelativeUrl(url) || !relativePath || !url) {
return url;
}
return relativePath + '/' + url;
});
}
}).catch(err => {
window.AnswersExperience.AnswersInitializedPromise.reject('Answers failed to initialized.');
Expand Down
22 changes: 12 additions & 10 deletions test-site/.gitignore
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
public/*
config/*
pages/*
cards/*
directanswercards/*
Gruntfile.js
package-lock.json
package.json
webpack-config.js

!cards/custom-cta-icons/
!config/global_config.json
!config/locale_config.json
!config/index.json
!pages/index.html.hbs
!public/iframe_test.html
!public/overlay.html
public/
config/
pages/
cards/
directanswercards/
Gruntfile.js
package-lock.json
package.json
webpack-config.js
!public/overlay.html
56 changes: 56 additions & 0 deletions test-site/cards/custom-cta-icons/component.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{{> cards/card_component componentName='custom-cta-icons' }}

class custom_cta_iconsCardComponent extends BaseCard['custom-cta-icons'] {
constructor(config = {}, systemConfig = {}) {
super(config, systemConfig);
}

dataForRender(profile) {
const linkTarget = AnswersExperience.runtimeConfig.get('linkTarget') || '_top';

return {
title: profile.name,
url: profile.website,
target: linkTarget,
titleEventOptions: this.addDefaultEventOptions(),
date: Formatter.bigDate(profile),
subtitle: Formatter.dateRange(profile),
details: profile.description,
CTA1: {
label: 'Hitchhiker Thumb!',
/**
* @Test a custom icon url without iconName in the CTA
* @Expect the custom icon to show up
*/
iconUrl: 'static/assets/hitchhiker-thumb.jpeg',
url: 'testUrl',
target: linkTarget,
eventType: 'RSVP',
eventOptions: this.addDefaultEventOptions(),
},
CTA2: {
label: 'AYAYA',
iconName: 'star',
/**
* @Test a custom icon url hen an iconName already exists
* @Expect the custom icon to show up
*/
iconUrl: 'static/assets/ayaya.png',
url: 'testUrl',
target: linkTarget,
eventType: 'DRIVING_DIRECTIONS',
eventOptions: this.addDefaultEventOptions(),
}
};
}

static defaultTemplateName (config) {
return 'cards/custom-cta-icons';
}
}

ANSWERS.registerTemplate(
'cards/custom-cta-icons',
{{{stringifyPartial (read 'cards/event-standard/template') }}}
);
ANSWERS.registerComponentType(custom_cta_iconsCardComponent);
8 changes: 8 additions & 0 deletions test-site/config-overrides/events_custom_cta_icons.es.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"verticalsToConfig": {
"events": {
"cardType": "custom-cta-icons",
"label": "Eventos"
}
}
}
7 changes: 7 additions & 0 deletions test-site/config-overrides/events_custom_cta_icons.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"verticalsToConfig": {
"events": {
"cardType": "custom-cta-icons"
}
}
}
2 changes: 1 addition & 1 deletion test-site/config/global_config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"sdkVersion": "1.9", // The version of the Answers SDK to use
"sdkVersion": "feature/icon-partial-i18n", // The version of the Answers SDK to use
"apiKey": "2d8c550071a64ea23e263118a2b0680b", // The answers api key found on the experiences page. This will be provided automatically by the Yext CI system
// "experienceVersion": "<REPLACE ME>", // the Answers Experience version to use for API requests. This will be provided automatically by the Yext CI system
// "businessId": "<REPLACE ME>", // The business ID of the account. This will be provided automatically by the Yext CI system
Expand Down
2 changes: 1 addition & 1 deletion test-site/config/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"mapConfig": {
"mapProvider": "Google"
},
"icon": "static/assets/slapshot.png"
"iconUrl": "static/assets/slapshot.png"
},
"products": {
"universalSectionTemplate": "grid-three-columns",
Expand Down
11 changes: 10 additions & 1 deletion test-site/scripts/create-verticals.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ const verticalConfiguration = {
template: 'vertical-standard',
cardName: 'event-custom'
},
events_custom_cta_icons: {
verticalKey: 'events',
template: 'vertical-standard'
},
faqs: {
verticalKey: 'faq',
template: 'vertical-standard',
Expand Down Expand Up @@ -90,7 +94,12 @@ const testSiteDir = path.resolve(__dirname, '..');
process.chdir(testSiteDir);

Object.entries(verticalConfiguration).forEach(([pageName, config]) => {
execSync(`npx jambo vertical --name ${pageName} --verticalKey ${config.verticalKey} --template ${config.template} --cardName ${config.cardName} --locales es`);
let verticalCommand =
`npx jambo vertical --name ${pageName} --verticalKey ${config.verticalKey} --template ${config.template} --locales es`
if (config.cardName) {
verticalCommand += ` --cardName ${config.cardName}`
}
execSync(verticalCommand);
configMerger.mergeConfigForPage(pageName);
pagePatcher.applyPatchToPage(pageName);
configMerger.mergeConfigForPage(pageName + '.es');
Expand Down
Binary file added test-site/static/assets/ayaya.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test-site/static/assets/hitchhiker-thumb.jpeg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
56 changes: 56 additions & 0 deletions tests/hbshelpers/relativePathHandler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import relativePathHandler from '../../hbshelpers/relativePathHandler';

it('works for undefined url', () => {
const hash = {
url: undefined,
relativePath: '.'
};
expect(relativePathHandler({ hash })).toEqual(undefined);
});

it('works for null url', () => {
const hash = {
url: null,
relativePath: '.'
};
expect(relativePathHandler({ hash })).toEqual(null);
});

it('works for blank string url', () => {
const hash = {
url: '',
relativePath: '.'
};
expect(relativePathHandler({ hash })).toEqual('');
});

it('works for relative url with relativePath', () => {
const hash = {
url: 'mySpecialAsset.jpg',
relativePath: '.'
};
expect(relativePathHandler({ hash })).toEqual('./mySpecialAsset.jpg');
});

it('works for relative url with relativePath that points back a level', () => {
const hash = {
url: 'mySpecialAsset.jpg',
relativePath: '../..'
};
expect(relativePathHandler({ hash })).toEqual('../../mySpecialAsset.jpg');
});

it('works when no relativePath', () => {
const hash = {
url: 'mySpecialAsset.jpg'
};
expect(relativePathHandler({ hash })).toEqual('mySpecialAsset.jpg');
});

it('works with absolute url', () => {
const hash = {
url: '/mySpecialAsset.jpg',
relativePath: '../../'
};
expect(relativePathHandler({ hash })).toEqual('/mySpecialAsset.jpg');
});
3 changes: 3 additions & 0 deletions tests/percy/photographer.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class Photographer {
await this._pageNavigator.gotoVerticalPage('events', { query: 'vrginia' });
await this._camera.snapshot('vertical-search--spellcheck');

await this._pageNavigator.gotoVerticalPage('events_custom_cta_icons');
await this._camera.snapshot('vertical-search--custom-cta-icons');

await this._pageNavigator.gotoVerticalPage('financial_professionals', { query: 'connor' });
await this._camera.snapshot('vertical-search--financial-professional-location');

Expand Down

0 comments on commit 2acdda1

Please sign in to comment.