-
Notifications
You must be signed in to change notification settings - Fork 615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(search): ES-97 fixed the html special chars issue in facet names #1637
Conversation
Autotagging @bigcommerce/storefront-team @davidchin |
@@ -30,7 +30,7 @@ <h5 class="accordion-title"> | |||
class="navList-action navList-action--checkbox {{#if selected }} is-selected {{/if}}" | |||
rel="nofollow" | |||
data-faceted-search-facet> | |||
{{ sanitize title }} | |||
{{{ title }}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also update the label across each of the facets.
@@ -15,7 +15,7 @@ <h5 class="sidebarBlock-heading"> | |||
{{#if facet '===' 'rating'}} | |||
{{lang 'search.faceted.selected.rating-label' rating=title}} | |||
{{else}} | |||
{{ title }} | |||
{{{ title }}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do it across all these files https://github.com/bigcommerce/cornerstone/tree/master/templates/components/faceted-search/facets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made changes to all applicable files. Please review them.
acf14cb
to
31bf694
Compare
31bf694
to
39d761d
Compare
@@ -7,7 +7,7 @@ | |||
rel="nofollow" | |||
data-id="{{ id }}" | |||
data-faceted-search-facet> | |||
{{title}} | |||
{{{title}}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably sanitize
all of the titles--there's little to no reason anyone should be using html in a custom field or option value name or value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added them.. pls review it.
39d761d
to
a19e13e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @junedkazi
What?
As part of fixing the encoding issues for ES-97, we found that html characters are being displayed in stencil theme. This fix will address it.
Tickets / Documentation
Screenshots (if appropriate)
Broken "RefineBy" facet names
Fixed "RefineBy" facet names