Skip to content

Commit

Permalink
full-page-map: fix alternative verticals iconUrl not using relativePa…
Browse files Browse the repository at this point in the history
…th (#873)

Noticed this while testing replacing all the IconComponent usages.

Relative path was not being appended to the iconUrl correctly
because the template was referencing just `relativePath`
and not `@root.relativePath` or `../../relativePath` 
(i.e. it was using the wrong handlebars context).

This wouldn't have caused visual issues for most users,
since with the relativePath not being set correctly the iconUrl
would get set to `/<iconUrl>`, instead of something like `../<iconUrl>`
which for most cases will point to the same location.

This only matters if somebody is taking the files output by the
theme's build, and moving them into a directory structure.
In that case things may break, since the two iconUrls above may
no longer point to the same location.

I changed the other two relativePath usages for consistency

J=SLAP-1297
TEST=manual

test that the iconUrl for alternativeverticals on a full page map
now uses the correct relative url, instead of the absolute url
that would result from relativePath being null

added a percy snapshot for alternative verticals on the full page map,
though this doesn't really test what was being fixed
  • Loading branch information
oshi97 authored Jul 7, 2021
1 parent 4d66757 commit ea24a5e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 4 deletions.
3 changes: 2 additions & 1 deletion test-site/config-overrides/people.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
},
"verticalsToConfig": {
"people": {
"label": "People"
"label": "People",
"iconUrl": "static/assets/ayaya.png"
}
}
}
4 changes: 4 additions & 0 deletions tests/percy/photographer.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ class Photographer {
await this._pageNavigator.click(mapboxPinSelector);
await this._camera.snapshotMobileOnly('vertical-full-page-map__mobile-detail-view');

await this._pageNavigator
.gotoVerticalPage('locations_full_page_map_with_filters', { query: 'people' });
await this._camera.snapshot('vertical-full-page-map--alternative-verticals');

await this._pageNavigator
.gotoVerticalPage('locations_full_page_map', { query: 'office sparce'});
await this._camera.snapshotDesktopOnly('vertical-full-page-map--spellcheck__desktop-view');
Expand Down
20 changes: 17 additions & 3 deletions theme-components/vertical-full-page-map/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,16 @@ ANSWERS.addComponent('VerticalFullPageMapOrchestrator', Object.assign({},
{{#with (lookup verticalsToConfig verticalKey)}}
{{#if isFirst}}isFirst: {{isFirst}},{{/if}}
{{#if icon}}icon: "{{{icon}}}",{{/if}}
{{#if iconUrl}}iconUrl: "{{#unless (isNonRelativeUrl iconUrl)}}{{relativePath}}/{{/unless}}{{{iconUrl}}}",{{/if}}
{{#if iconUrl}}iconUrl: "{{{relativePathHandler url=iconUrl relativePath=@root.relativePath}}}",{{/if}}
label: {{> verticalLabel overridedLabel=label verticalKey=../verticalKey fallback=@key}},
url: "{{#if url}}{{{url}}}{{else if ../url}}{{../../relativePath}}/{{{../url}}}{{else}}{{{@key}}}.html{{/if}}",
url:
{{#if url}}
"{{{relativePathHandler url=url relativePath=@root.relativePath}}}",
{{else if ../url}}
"{{{relativePathHandler url=../url relativePath=@root.relativePath}}}",
{{else}}
"{{{@key}}}.html",
{{/if}}
{{/with}}
}{{#unless @last}},{{/unless}}
{{else}}
Expand All @@ -43,7 +50,14 @@ ANSWERS.addComponent('VerticalFullPageMapOrchestrator', Object.assign({},
{{#if isFirst}}isFirst: {{isFirst}},{{/if}}
{{#if icon}}icon: "{{{icon}}}",{{/if}}
label: {{#if label}}"{{{label}}}"{{else}}"{{{@key}}}"{{/if}},
url: "{{#if url}}{{{url}}}{{else if ../url}}{{../../relativePath}}/{{{../url}}}{{else}}{{{@key}}}.html{{/if}}",
url:
{{#if url}}
"{{{relativePathHandler url=url relativePath=@root.relativePath}}}",
{{else if ../url}}
"{{{relativePathHandler url=../url relativePath=@root.relativePath}}}",
{{else}}
"{{{@key}}}.html",
{{/if}}
{{/with}}
}{{#unless @last}},{{/unless}}
{{/if}}
Expand Down

0 comments on commit ea24a5e

Please sign in to comment.