yeswework / fabrica-dev-kit

A toolkit for faster, smoother WordPress 5 development
https://fabri.ca/
MIT License
274 stars 27 forks source link

Twig compiles wrongly to theme folder #14

Closed matteodem closed 7 years ago

matteodem commented 7 years ago

Just found out that the compiler from the dev/src folder to the wordpress theme transforms special if statements incorrectly.

Current:

dev/src twig file

<div
  {% if doNotMobileImage %}
    data-mobilewidth="{{ TimberImage(images_mobile).width }}"
    data-mobileheight="{{ TimberImage(images_mobile).height }}"
  {% endif %}
></div>

wp-content/themes twig file

<div {%="" if="" doNotMobileImage="" %}="" data-mobilewidth="{{ TimberImage(images_mobile).width }}" data-mobileheight="{{ TimberImage(images_mobile).height }}" ...

Expected:

wp-content/themes twig file same as dev/src twig file.

andrewstaffell commented 7 years ago

Hi @matteodem... it's a known issue caused by the PostHTML-BEM preprocessor, which interprets anything within an opening tag declaration as an attribute-value pair and renders it accordingly.

Solutions: 1) If you want to use PostHTML-BEM to be able to code with BEM attributes (block, elem, mods), basically you need to make sure any Twig code is inside the "value" part of an attribute-value pair, ie. between the double quotes. So this should work:

{% if doNotMobileImage %}
{% set mobileWidth = TimberImage(images_mobile).width %}
{% set mobileHeight = TimberImage(images_mobile).height %}
{% endif %}
<div data-mobilewidth="{{ mobileWidth }}" data-mobileheight="{{ mobileHeight }}"></div>

(If you're really using an empty <div> you could just put the whole tag between the {% if %} clause, but I assume you simplified the markup for the issue?)

2) If you don't intend to use PostHTML-BEM, just comment out the corresponding line in dev/gulpfile.js (it's line 169, within views(), in the current repo version) as follows, and your existing code should work fine. // .pipe(posthtml([posthtmlBem(options.posthtmlBem)]))

Let me know how you get on!

andrewstaffell commented 7 years ago

Better version of the suggested code in solution 1 above:

{% set mobileWidth = doNotMobileImage ? TimberImage(images_mobile).width : '' %}
{% set mobileHeight = doNotMobileImage ? TimberImage(images_mobile).height : '' %}
<div data-mobilewidth="{{ mobileWidth }}" data-mobileheight="{{ mobileHeight }}"></div>
matteodem commented 7 years ago

The suggested code creates different output: empty data attributes instead of no data attributes.

andrewstaffell commented 7 years ago

That's true, if that's no good then I think you'd need to wrap the whole tag in the conditional, like:


{% if doNotMobileImage %}
{% set mobileWidth = TimberImage(images_mobile).width %}
{% set mobileHeight = TimberImage(images_mobile).height %}
<div data-mobilewidth="{{ mobileWidth }}" data-mobileheight="{{ mobileHeight }}"></div>
{% else %}
<div></div>
{% endif %}```
matteodem commented 7 years ago

Is this bug fixed? Having a work-around is good for now, but it's not the solution to this problem.

andrewstaffell commented 7 years ago

Hi @matteodem. It's not a bug but an inherent limitation of the PostHTML-BEM library. There's nothing we can change within FDK to correct it. You could pursue the matter by raising an Issue (or even submitting a PR!) at https://github.com/rajdee/posthtml-bem. Although I think we previously did some investigation and concluded that some Twig structures would be ambiguous for the BEM parser, so maybe it's not possible to fix it programmatically, but not totally sure about that.

The workaround does require a bit more code, but since it's possible to get the exact desired output, it's still a valid solution.