pre work for replacing IconComponent usages with iconPartial #870
Conversation
- 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 J=SLAP-1297 TEST=manual,auto test that custom icons still work, both for section titles on universal and inside cards
| @@ -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) { | |||
There was a problem hiding this comment.
It's kind of crummy that we have to basically duplicate isNonRelativeUrl and relativePathUrlHandler. It'd be great if the JS in hbshelpers could be inlined as a partial here. Would save us having to update the same code-block twice. Is there any non-janky way to do that?
There was a problem hiding this comment.
You know, at first I thought this was going to be hard because it's hard to get the path from entry.js to the hbshelpers folder. That path depends on the jambo config, and we don't want to use dynamic imports here (with like a require.resolve or something). BUT I'm remembering now that webpack lets you add resolve aliases. Will add in a separate PR.
There was a problem hiding this comment.
I lied. Webpack doesn't like it when you mix ES6 and commonjs, and unfortunately jambo only understands hbs helpers that are commonjs. So putting it into the bundle is probably out of the window for now.
I tried thinking about how to do the inlining method as well with just HBS, but obviously module.exports is not something hbs understands. We could do jank where we create an empty object called "module", then loop over the desired helpers, continually reassigning the value of module.exports and calling ANSWERS.registerHelper on it. jank !== not janky though
There was a problem hiding this comment.
sad developer noises this was the pr #871
For whatever reason it works fine with our dev webpack build, just borks on prod ("it" being the first commit before I started trying random ideas)
| @@ -1,15 +1,17 @@ | |||
| public/* | |||
There was a problem hiding this comment.
for reasons apparently git is finicky and whether or not you put a star at the end of a path matters
https://stackoverflow.com/questions/5533050/gitignore-exclude-folder-but-include-specific-subfolder
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
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.
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
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