add title logic in the <head> #87
Conversation
layout/theme.liquid
Outdated
| if request.page_type == 'collection' | ||
| for filter in collection.filters | ||
| for value in filter.values | ||
| if value.active | ||
| echo ' / ' | ||
| echo value.label | ||
| endif | ||
| endfor | ||
| endfor | ||
| endif |
There was a problem hiding this comment.
Added this to see how it would work for the filtering to show up. I would still need to add something in the JS to update the page title as it updates the updateURLHash() if we decide to do it this way.
There was a problem hiding this comment.
Seems really granular to have theme devs update the title like this, especially as we release new features that could potentially impact it, e.g. collection filtering. I would remove for now until we have a better general sense of how we want to improve our approach to titles in our themes.
layout/theme.liquid
Outdated
| {%- liquid | ||
| capture seo_title | ||
| if request.page_type == 'search' and search.performed == true | ||
| if search.results_count == 1 |
There was a problem hiding this comment.
We don't need this condition, you can leverage pluralized translations.
layout/theme.liquid
Outdated
| if request.page_type == 'collection' | ||
| for filter in collection.filters | ||
| for value in filter.values | ||
| if value.active | ||
| echo ' / ' | ||
| echo value.label | ||
| endif | ||
| endfor | ||
| endfor | ||
| endif |
There was a problem hiding this comment.
Seems really granular to have theme devs update the title like this, especially as we release new features that could potentially impact it, e.g. collection filtering. I would remove for now until we have a better general sense of how we want to improve our approach to titles in our themes.
|
Something else we need to think about is updating scripts like the facets to change the number of items in the title whenever the page updates as well, along with anywhere else that the page state updates and should reflect in the title. Not sure about this one, but maybe the product page having the variant name instead of just the product name as well? That would need updating when options are selected as well. Not sure if this is desired. |
Why are these changes introduced?
About -> #77 .
What approach did you take?
Added logic to have something similar to what's shown in the issue
Other considerations
Should we also add the filtering tags in the title ? When looking at Debut, it had:
{%- if current_tags -%} {%- assign meta_tags = current_tags | join: ', ' -%} – {{ 'general.meta.tags' | t: tags: meta_tags -}} {%- endif -%}Since now we're using a different filtering approach, it would need some new logic.
I could do something like this:
if request.page_type == 'collection' for filter in collection.filters for value in filter.values if value.active echo ' – ' echo value.label endif endfor endfor endifAnd I would need to update the JS for filters so it updates the
titlethe same way it currently updates theurlwithupdateURLHash().In the desired output above, we would add
Errorin the title to reflect an error in form submission.form.errorsis something that can only be used within a form. I could use it to do an assign and check against that assign intheme.liquidbut that seem like a scrappy solution.It would be ideal if we didn't have to hard code the logic for the page title as it usually won't differ from theme to theme. I'm curious to hear what @Thibaut thinks about it.
Demo links
Checklist