Shopify / theme-tools

Everything developer experience for Shopify themes
https://shopify.dev/docs/themes
MIT License
72 stars 19 forks source link

{%- render -%} autocomplete #475

Open stijns96 opened 1 month ago

stijns96 commented 1 month ago

Is your feature request related to a problem? Please describe. I always have to check the self written documentation in my snippets to see what props I can pass to my snippet.

Describe the solution you'd like I was thinking about something like this.

{% prop source = object | required: true %}
{% prop width = number | required: true | default: 768 %}

{%- liquid
  prop source = object | required: true
  prop width = number | required: true | default: 768
-%}

I know this image now shows the source autocomplete from HTML, but this could be an autocomplete/suggestion from Liquid. It works a little bit like typescript, but then for liquid.

Image

At this point I'm not sure if this will work for liquid, but it's just a suggestion that I'd like to see.

Checklist

karreiro commented 1 month ago

👋 Hi @stijns96, thanks for the suggestion!

This is an excellent idea, and I'll take it back to the team for discussion. I'll update this issue as soon as we have any updates.

karreiro commented 1 month ago

Dawn snippets follow a strong convention of defining accepted elements in the first comment of the file. I believe we can take advantage of this in a JSdocs fashion, as this is a purely tooling-focused feature. Additionally, we can use the comments to provide inline documentation while folks type, similar to the assistance provided with objects.

{% prop source = object | required: true %}
{% prop width = number | required: true | default: 768 %}

{%- render 'test', soâ–ˆ -%}
                     ^-- code completes with source
{% comment %}
  Renders the test snippet

  Accepts:
  - source: {object} Blog object
  - width: {number} Width of the page

  Usage:
  {% render 'test', source: 'the source', width: 800 %}
{% endcomment %}

{%- render 'test', soâ–ˆ -%}
                     ^-- code completes with source

This issue relates to this one: https://github.com/Shopify/theme-tools/issues/114. While we're more focused on the render tag, it's valuable to keep that in mind :)

stijns96 commented 1 month ago

So if I understand it correctly, we don’t want to create a new variable like prop, but we want to use the first comment for this? Although I like this technique, I think it comes with some limitations or we have to come op with a slightly different approach.

The limitation I see is that we can not easily add more requirements to the properties we want to pass. This idea actually came up when I was working on a Vue project. See their props documentation here.

I ones created an extended docs for a quite extensive image snippet.

{%- comment -%}
  Files must meet the following requirements:
    https://help.shopify.com/en/manual/shopify-admin/productivity-tools/file-uploads

  # Parameters:       | Required    | Type                | Description                               | Accepted values
    - source          | true        | { Object }          | The image to be displayed.                |
    - class           | false       | { String }          | The class for the image.                  |
    - alt             | false       | { String }          | The alt text for the image.               |
    - title           | false       | { String }          | The title attribute for the image.        |
    - aspect_ratio    | false       | { String / Number } | The aspect ratio of the image.            | 'x/y' or 'x.y'
    - max_width       | false       | { String / Number } | The maximum width of the image.           | '1000' or 1000
    - lazy            | false       | { Boolean }         | Whether the image should be lazy loaded.  | true or false
    - fetchpriority   | false       | { String }          | The fetch priority for the image.         | 'low' or 'high'
    - sizes           | false       | { String }          | The sizes attribute for the image.        | One or more strings separated by commas, indicating a set of source sizes ordered from large to small. Each source size consists of:
                                                                                                          1. A media condition. This must be omitted for the last item in the list.
                                                                                                          2. A source size value.
    - widths          | false       | { String }          | The widths attribute for the image.       | 'xxx, xxx, xxxx ...'
    - figcaption      | false       | { String }          | The caption for the image.                |
    - fallback_image  | false       | { String }          | The fallback image for the image.         | https://shopify.dev/docs/api/liquid/filters/placeholder_svg_tag

  # Recommendations:
    - The max_width parameter is recommended to ensure the srcset is not larger than the image.
    - The sizes parameter is recommended to ensure the image is displayed responsively.
    - Only use fetchpriority 'high' for images that are critical to the user experience (Above the fold).

  # Example:
    {%- render 'format-image',
      source: section.settings.image,
      lazy: true,
      sizes: '(min-width: 1200px) 50vw, 100vw'
    -%}
{%- endcomment -%}

It could also be something like this.

{%- comment -%}
  Files must meet the following requirements:
    https://help.shopify.com/en/manual/shopify-admin/productivity-tools/file-uploads

  # Accepts:
    - source
      required:       true
      type:           Object
      description:    The image to be displayed.

    - aspect_ratio
      required:       false
      type:           String / Number
      description:    The aspect ratio of the image.
      acceted values: 'x/y' or 'x.y'

  # Recommendations:
    - The max_width parameter is recommended to ensure the srcset is not larger than the image.
    - The sizes parameter is recommended to ensure the image is displayed responsively.
    - Only use fetchpriority 'high' for images that are critical to the user experience (Above the fold).

  # Usage:
    {%- render 'format-image',
      source: section.settings.image,
      lazy: true,
      sizes: '(min-width: 1200px) 50vw, 100vw'
    -%}
{%- endcomment -%}

I still think this could bloat the snippet quite a bit... Imagine something like this where the prop variable also works as an assign.

{%- liquid
  prop source = object | required | description: 'The image to be displayed.'
  prop aspect_ratio = string or number | description: 'The aspect ratio of the image.' | accepted_values: 'x/y' or 'x.y'
-%}

{%- prop source = object
  | required
  | description: 'The image to be displayed.'
-%}

{%- prop aspect_ratio = string or number
  | description: 'The aspect ratio of the image.'
  | accepted_values: 'x/y' or 'x.y'
  | default: 1.4
-%}

{{ aspect_ratio }} -> 1.4

Both have their up and downsides, but the possibility to make values required etc, can give nice theme checks as well.