liquid icon indicating copy to clipboard operation
liquid copied to clipboard

Documentation: Organize filters into single-page categories by their general use case

Open ADTC opened this issue 3 years ago • 6 comments

@dylanahsmith @pushrax thank you for approvals and merging #1408. I wish to contribute another PR for better organizing the filters in the official docs, where it is currently a long alphabetical list of all types of filters, and each filter has its own page. Compare this to Shopify's version:

  1. Filters are categorized by their general use case.
  2. All filters within a single category are placed on a single page.
  3. There's an introduction on the page before the first filter.

image

If it is something you will accept, I'll work on reorganizing the documentation of filters to a similar format. 🙂

ADTC avatar Mar 08 '21 17:03 ADTC

There are trade-offs to having one or the other. Grouping related filters or tags by category helps with discovery when the reader doesn't know what is available. On the other hand, the reader might want reference documentation for a tag or filter where they know the name, such as when they see it used in a template written by someone else.

Right now the documentation seems inconsistent by having tags grouped by category and filters not grouped.

Perhaps we could solve both use cases by having tags and filters grouped by category in the sidebar, while also having an index page that links to all tags and/or filters to use for reference documentation.

dylanahsmith avatar Mar 08 '21 17:03 dylanahsmith

Yes, I realized that with the current setup, each filter gets an URL that isn't an anchor link. I think we will be fine if we can redirect these page based URLs to anchor link based URLs, so that old references do not break. We can have the same setup for tags.

For example:

https://shopify.github.io/liquid/filters/abs/

redirects to:

https://shopify.github.io/liquid/filters/math/#abs

Is such redirection possible?

It's also necessary to have a summary listing of each subheading in the page. In Shopify, this appears on the right side. This might be a good setup to replicate.

Also: Tags section needs some restructuring as well. Raw and Comment pages should be removed, and replaced with a single Template page, which documents all of these:

comment
echo
include
liquid
raw
render

And the ifchanged tag needs to be documented under Control Flow or Iteration (which one?). This documentation looks good but I'm not sure if we'd have permission to copy.

Finally, aren't there any objects we can document? The forloop object comes to mind.

ADTC avatar Mar 08 '21 18:03 ADTC

Good point about redirects. It looks like we can use anchor redirects. For instance, I tried changing the https://shopify.github.io/liquid/tags/ redirect to instead redirect to https://shopify.github.io/liquid/tags/control-flow/#unless locally as a proof of concept and it worked with this change

diff --git a/_tags/control-flow.md b/_tags/control-flow.md
index 83f30fc3..3a1a7651 100644
--- a/_tags/control-flow.md
+++ b/_tags/control-flow.md
@@ -1,7 +1,6 @@
 ---
 title: Control flow
 description: An overview of control flow and conditional tags in the Liquid template language.
-redirect_from: /tags/
 ---
 
 Control flow tags can change the information Liquid shows using programming logic.
diff --git a/pages/redirects/tags/index.html b/pages/redirects/tags/index.html
new file mode 100644
index 00000000..61b07276
--- /dev/null
+++ b/pages/redirects/tags/index.html
@@ -0,0 +1,4 @@
+---
+permalink: /tags
+redirect_to: /tags/control-flow/#unless
+---

It's also necessary to have a summary listing of each subheading in the page. In Shopify, this appears on the right side. This might be a good setup to replicate.

:+1: Just pay attention to how it would look on mobile

Also: Tags section needs some restructuring as well. Raw and Comment pages should be removed, and replaced with a single Template page

Yeah, those single tag pages seemed out of place.

And the ifchanged tag needs to be documented under Control Flow or Iteration (which one?).

It doesn't do any iteration, so Control Flow seems the most suitable.

This documentation looks good but I'm not sure if we'd have permission to copy.

They would have to provide a license giving permission to distribute a copy of it.

Finally, aren't there any objects we can document? The forloop object comes to mind.

It probably makes sense to document the forloop object along with the for tag. Another one is tablerowloop for the tablerow tag. These only make sense inside these tags, so it doesn't make sense to have independent object documentation.

dylanahsmith avatar Mar 08 '21 20:03 dylanahsmith

Great! Noted everything.

It looks like, the object should be tablerow according to this and not tablerowloop. Can I get a confirmation?

Most of the objects listed here are Shopify-specific but I'm suspecting request and template could be standard Liquid objects. Is there a code block where this can be verified?

PS: Is include officially deprecated in favor of render? Or is that just a Shopify thing?

ADTC avatar Mar 09 '21 01:03 ADTC

It looks like, the object should be tablerow according to this and not tablerowloop. Can I get a confirmation?

Looks like that documentation is wrong, I was using the source code as a reference (https://github.com/Shopify/liquid/blob/v5.0.0/lib/liquid/tags/table_row.rb#L36-L37)

Most of the objects listed here are Shopify-specific but I'm suspecting request and template could be standard Liquid objects. Is there a code block where this can be verified?

We shouldn't assume there is a request. For instance, jekyll pre-renders the templates since it is static site generator, so wouldn't have the concept of a request.

Shopify's template.name and template.suffix would probably have surprising behaviour outside the context of a Shopify template, since a template filename product.alternate.liquid would have a template.name of product and template.suffix of alternate. Anyways, the template object isn't builtin to liquid.

PS: Is include officially deprecated in favor of render? Or is that just a Shopify thing?

What is Shopify specific is that we actually restricted the use of the include tag in sections when introducing them. So it is officially deprecated in Shopify.

For liquid in general, I'm not sure how helpful it is to talk about whether or not it is deprecation. The more important point is to recommend the render tag since it avoid unintended side-effects. If there are intended side-effects, as brought up in #1288, then the include tag can still be used for that purpose, since there isn't any plan to remove it in the foreseeable future.

dylanahsmith avatar Mar 12 '21 00:03 dylanahsmith

I should note that there's a major documentation update ongoing in #1403 and we should avoid duplicate work and conflicts.

ADTC avatar Mar 26 '21 19:03 ADTC

And the ifchanged tag needs to be documented under Control Flow or Iteration (which one?). This documentation looks good but I'm not sure if we'd have permission to copy.

Looks like this is still missing. I see #1403 is closed so it shouldn't block further work.

microalps avatar Apr 21 '23 11:04 microalps