Shopify / theme-check

The Ultimate Shopify Theme Linter
Other
337 stars 95 forks source link

`RemoteAsset` incorrectly flagged on `source` element. #538

Closed ConduciveMammal closed 2 years ago

ConduciveMammal commented 2 years ago

Using media.sources, theme-check incorrectly flags source.url as an off-Shopify hosted asset.

<video
  playsinline
  {% if video_looping %}loop{% endif %}
  {% if video_style == 'muted' %}muted{% endif %}
  {% if video_style == 'unmuted' %}controls{% endif %}
  controlsList="nodownload"
  poster="{{ media.preview_image | image_url: width: 740 }}"
  data-image-count="{{ product.media.size }}"
  data-video-type="{{ video_type }}"
  data-video-style="{{ video_style }}"
  id="ProductVideo-{{ section_id }}-{{ loopIndex }}"
  class="product__video">
  {%- for source in media.sources -%}
    <source src="{{ source.url }}" type="{{ source.mime_type }}">
  {%- endfor -%}
  Your browser does not support the video tag.
</video>

image

charlespwd commented 2 years ago

Yeah implementation right now makes sure there's at least an asset_url filter applied to variables. Should probably revisit and accept variable lookups.

charlespwd commented 2 years ago

Looks like you're right but any reason why you're not using one the Media filters? (e.g. video_tag)

Seems like it can save you some trouble and bake in best practices for you :D

Probably something to do with the code you pasted :see_no_evil: I should have scrolled up a bit more :P