Skip to content
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

Fix content width when menubar_toc is enabled #85

Merged
merged 2 commits into from Jan 17, 2021

Conversation

wangjunlem
Copy link
Contributor

While menubar_toc is active, the content width is still set as is-12. This should fix the issue.

While `menubar_toc` is active, the content width is still set as `is-12`. This should fix the issue.
@chrisrhymes
Copy link
Owner

Thanks for this pull request @wangjunlem. When I test this out I get a liquid syntax error. It seems that the brackets are not allowed in liquid.

In tags with more than one and or or operator, operators are checked in order from right to left. You cannot change the order of operations using parentheses — parentheses are invalid characters in Liquid and will prevent your tags from working.
https://shopify.dev/docs/themes/liquid/reference/basics/operators

I find this extremely confusing to be honest. I've tried rewriting the if statement but would appreciate some help testing if possible?

{% if page.show_sidebar and page.menubar or page.menubar_toc %} 
    {% assign content_width = 'is-4' %} 
{% elsif page.menubar or page.menubar_toc or page.show_sidebar %} 
    {% assign content_width = 'is-8' %} 
{% else %} 
    {% assign content_width = 'is-12' %} 
{% endif %}

@wangjunlem
Copy link
Contributor Author

wangjunlem commented Jan 17, 2021

Sorry, I submitted this on the fly (which also explains the horrible branch name...)

Actually, I might have an alternative - without chaining operators and having to rely on heading back to the documentation every time we want to make sense of it, we could introduce another variable at the top, just so that a year down the line no one has to scratch their head:

{% if page.menubar or page.menubar_toc %}
    {% assign has_left_sidebar = true %}
{% endif %}

{% if page.show_sidebar and has_left_sidebar  %} 
    {% assign content_width = 'is-4' %} 
{% elsif page.show_sidebar or has_left_sidebar %} 
    {% assign content_width = 'is-8' %} 
{% else %} 
    {% assign content_width = 'is-12' %} 
{% endif %}

I've just checked it and this works.

(Funnily enough, my compiler didn't flag me for the Liquid syntax error...)

@chrisrhymes chrisrhymes merged commit bad69ae into chrisrhymes:master Jan 17, 2021
@chrisrhymes
Copy link
Owner

Thanks, this will make it much easier to maintain and see what is going on in future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants