Feat/templates filter macro #1877
Conversation
…uld be used. The templates list will need to be marked up with some new data attributes in order to make use of this functionality.
🧪 Review environmenthttps://ihyrlf3yxflvlf3topefmu72p40tsekv.lambda-url.ca-central-1.on.aws/ |
| <div class="template_filter" data-row-selector="{{ row_selector }}"> | ||
| <details> | ||
| <summary class="cursor-pointer text-sm">Apply filters</summary> | ||
| <div class="flex p-0 pl-gutter mt-2"> |
There was a problem hiding this comment.
Here, we can use gap-gutter withflex and remove ml-gutter from the children elements. This way we'll have an easier time making this responsive and potentially adding more filters later on!
🧪 Review environmenthttps://mie5xzhown76th472cxpv64tte0qtitu.lambda-url.ca-central-1.on.aws/ |
- fix aria-describedbys - fix duplicate ids - fix invalid heading structure - remove unneccessary and broken labelledby
Co-authored-by: Philippe Caron <[email protected]>
Co-authored-by: Philippe Caron <[email protected]>
Co-authored-by: Philippe Caron <[email protected]>
| {# Ensure all required parameters are provided #} | ||
| {% if not row_selector or not notification_type_data_attribue or not template_category_data_attribute %} | ||
| <div class="text-red font-bold my-4"> | ||
| Missing required parameters: <code>row_selector</code>, <code>notification_type_attribute</code>, and <code>template_category_attribute</code>. Please verify your code and try again. |
There was a problem hiding this comment.
Is this error message is targeted at devs, and not users? Are we able to log this instead of rendering it on the page on the off chance something goes awry? It could be confusing to a user if they were somehow exposed to it.
There was a problem hiding this comment.
Yes this is for devs. Solid point about accidentally showing this to users. I could add a check here to make sure its running locally.
There was a problem hiding this comment.
I what conditions would this show up? And what would a user need to do to fix it? Unless nothing can be done from the user's perspective, we could design an empty state component for it.
Otherwise if it is just for admins (us), we could wrap this around a permission conditional.
There was a problem hiding this comment.
Yeah i just added this so if a dev in the future tried to use it and had issues it could help them. The permission conditional is also a cool idea!
| @@ -29,7 +29,7 @@ | |||
| <legend class="sr-only">{{ _("Select templates and folders")}}</legend> | |||
| {% endif %} | |||
| {% for item in template_list %} | |||
| <div class="template-list-item {% if display_checkboxes %}template-list-item-with-checkbox{% endif %} {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}"> | |||
| <div class="template-list-item {% if display_checkboxes %}template-list-item-with-checkbox{% endif %} {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}" data-notification-type="{{ item.hint }}" data-template-category="{{ item.template_category[template_category_name_col] }}" data-testid="template-row"> | |||
There was a problem hiding this comment.
Less of a suggestion, more of a discussion (that maybe we have elsewhere):
{{ 'template-list-item-with-checkbox' if display_checkboxes }} is functionally the same as {% if display_checkboxes %}template-list-item-with-checkbox{% endif %}. Thoughts on using expressions for simple inline conditionals for readability / maintainability?
There was a problem hiding this comment.
I think what you are proposing is way better than what we have here. I'll update it and keep this in mind going forward!
| @@ -0,0 +1,128 @@ | |||
| {# | |||
| This macro creates a filterable template list based on specified criteria. | |||
There was a problem hiding this comment.
I love this docstring so much!
| <div class="template-filter" data-row-selector="{{ row_selector }}"> | ||
| <details data-testid="filter"> | ||
| <summary data-testid="filter-toggle">{{ _("Apply filters") }}</summary> | ||
| <nav class="flex p-0 gap-gutter pl-gutter mt-2" aria-label="{{ _('Filter by template type and category') }}" data-testid="filter-content"> |
There was a problem hiding this comment.
We can remove pl gutter here to keep things aligned.
| <nav class="flex p-0 gap-gutter pl-gutter mt-2" aria-label="{{ _('Filter by template type and category') }}" data-testid="filter-content"> | |
| <nav class="flex p-0 gap-gutter mt-2" aria-label="{{ _('Filter by template type and category') }}" data-testid="filter-content"> |
Summary | Résumé
This PR:
FF_TEMPLATE_CATEGORY=trueUsing the macro requires a few annotations on the template list:
data-notification-type="Email message"data-template-category="Status Update"