Meta create components

Not sure form components are needed?

Some feedbacks

breadcrumb

• items field to convert to links setting type once ✨ Add links setting type Needs review will be ready
• I don't understand the need of {% set nav_attributes = create_attribute() %}
• Don't hardcode the variant ID here addClass('usa-breadcrumb--wrap')

button

OK

button_group

• Don't hardcode the variant ID here addClass('usa-button-group--segmented')

collection

Items is a "rendable array of items" but the preview has only one item. So:

• maybe the description is wrong, and the preview must not be a sequence
• maybe a second item is expected in the preview

Also, careful with "rendable" typo

You don't use the default attributes object but the custom list_attributes prop instead. Why?

collection_item & collection_item_meta_tags

I have the feeling all those components (collection, collection_item, collection_item_meta_tags...) can be merged and simplified.

Because, they have many issues and it may be the result of overengineering:

Testing the render array structure in template may be harmful:

{% if meta_items_list['#sources'] is defined %} 
...
{% if meta_items_list['#type'] is defined %}

Normally, slots content are opaque.

In collection_item_meta_tags

• We are not using the attributes object here.
• Don't do premature rendering: meta_item|render|striptags|trim

combo_box

options field is not a slot, according to its preveiw which is not a renderbale but a mapping:

        - text: "Apple"
        - text: "Apricot"
        - text: "Banana Smoothie"

if access_label is an ID, you can use the new "machine_name" setting type instead of textfield: ✨ Add machine_name setting type Fixed

in_page_navigation

We are not using the attributes object in the template

You are free to pick the component ID you want, but my advice is to use the block (as in BEM) unprefixed class name, so in-page-nav instead of in-page-nav-container

summary_box

We are not using the attributes object in the template

Do we allow the printing of empty <h3 class="usa-summary-box__heading"></h3> ? if not, let's add a condition.

Summary heading is a slot (field) so this is risky:
{% set id = summary_heading|lower|replace({' ': '_'}) %}

I would suggest to:

1. add a "summary_box_id" machine_name prop (UI Patterns 1.x doesn't allow prop named "id" which is a reserved word. UI Patterns 2.x will allow this)
2. set a default value in the template: {% set summary_box_id = summary_box_id|default('"summary-box--' ~ random()) %}
3. use the full value in the attribute: <h3 class="usa-summary-box__heading" id="{ summary_box_id }}">

tag

Don't hardcode the variant ID here addClass('usa-tag--big')

side_navigation

• items field to convert to links setting type once ✨ Add links setting type Needs review will be ready
• one those items converted, it will be the opportunity to remove manipulations of the URL obejcts (getOption, isRouted...)
• the template is not using the attribute object

social_link

• href is not a slot (field) but an URL prop (setting)
• according to the template, name is not a slot (field) but a string prop (textfield setting)
• the template is not using the attribute object
• is .grid-col-auto wrapper really part if the component?

social_links_group

I am not sure this component is expected...

language_selector

language_label must not be a field because it is not a slot, but a string prop (textfield setting), which is printed in an attribute value.

items can stay a field for now, but let's convert it to links setting type once ✨ Add links setting type Needs review .

I guess item.langcode must be item.attributes.langcode

list

row field is a mapping with attributes and content properties. So, it doesn't look like a proper slot. If you don't need the attribute property (i guess it is the case according to the upstream doc), you can replace the mapping by a list of renderable.

If you absolutely need attributes, i don't have solution to propose yet, but I would be happy to discuss it.

accordion and accordion_item

OK, nothing to say :)

Side note: https://designsystem.digital.gov/components/accordion/ is a bit weird with no wrapper around accordions items. That's something interesting about implementing design systems components: mastly the same components, but never the same ways.

modal

You don't need the complicated variant management if there is nor "__" string neither "_" in your variants ID.

So, instead of this:

{% if variant and variant|lower != 'default' %}
  {% set variants = variant|split('__')|map(v => v|lower|replace({(v): 'usa-modal--' ~ v})|replace({'_': '-'})) %}
  {% set attributes = attributes.addClass(variants) %}
{% endif %}

You can have something like that (non tested code):

  {% set attributes = (variant and variant != 'default') ? attributes.addClass('usa-modal--' ~ variant})) : attributes %}

Pushed the fix for modal.

Moved the following to the final approved list :)

breadcrumbs
buttons
button groups
in page nav
Modal
Summary box
Tag

Getting there!

Believe all patterns are ready for review now!!

Ok, I do the review :)

Before doing a proper manual review, I have run a little script to check callables (functions, macros, methods) and variables usages in templates.

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: slim >> It may be a variant

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes
• Some variables are not defined: uswds_images >> it is related to ui_suite_uswds_preprocess hook but such hooks are forbidden in SDC & UI Patterns 2. We have to find another way.

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: checkbox >> You are the first to componentize form elements, what you do will be interesting for UI Patterns and SDC communities, but we have quirks to avoid

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: variant >> if they are only one variant, there are no variant

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not defined: disbaled >> a typo for "disabled"

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: date_picker >> You are the first to componentize form elements, what you do will be interesting for UI Patterns and SDC communities, but we have quirks to avoid

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: radio >> You are the first to componentize form elements, what you do will be interesting for UI Patterns and SDC communities, but we have quirks to avoid

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: slider >> You are the first to componentize form elements, what you do will be interesting for UI Patterns and SDC communities, but we have quirks to avoid

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not defined: uswds_images

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not defined: name_value

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not defined: uswds_images >> it is related to ui_suite_uswds_preprocess hook but such hooks are forbidden in SDC & UI Patterns 2. We have to find another way.

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not defined: header_columns

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: time_picker >> You are the first to componentize form elements, what you do will be interesting for UI Patterns and SDC communities, but we have quirks to avoid

https://git.drupalcode.org/project/ui_suite_uswds/-/tree/3.0.x/templates...

• Some variables are not used: attributes

Not tested: modal, pagination, input_mask

In https://git.drupalcode.org/project/ui_suite_uswds/-/blob/3.0.x/templates...

you do :

          {% do item.attributes.addClass('usa-current') %}
          {% do item.attributes.setAttribute('aria-current', 'page') %}

This is possible because of a "bug" in Drupal attributes object manipulation from Twig template: #3296456: Twig attribute setter methods are not immutable →

According to Twig/Jinja good practices, it is better to do something like that:

          {% set item_attributes = item.attributes %}
          {% set item_attributes = item_attributes.addClass('usa-current').setAttribute('aria-current', 'page')  %}

In https://git.drupalcode.org/project/ui_suite_uswds/-/blob/3.0.x/templates...

Variants ID are integer, and they are attached to 'usa-icon--size-'

So, they look more like values of a numeric "size" prop than variants.

Addressed most of the feedback in https://git.drupalcode.org/project/ui_suite_uswds/-/commit/801dfbd66cfc8...

All except

Some variables are not defined: uswds_images >> it is related to ui_suite_uswds_preprocess hook but such hooks are forbidden in SDC & UI Patterns 2. We have to find another way.

Not sure how to get around this one, unless I add a setting to just pass in the variable.

Great.

Not sure how to get around this one, unless I add a setting to just pass in the variable.

You are not the first one with this need/issue. Let's talk about that in the slack channel.

Other feedbacks.

Variants management

Button is the only component with "__" in variants ID, but this snippet:

is also found in the templates of those components:

• alert
• icon
• search
• step_indicator
• tooltip

collection_item_meta & collection_item_meta_tags

{% for meta_item in meta_items %}
{% if meta_item|render|striptags|trim %}
...
{% if meta_items|render|striptags|trim %}
..

Meta items is a slot, so doing {% if meta_items %} must be enough.

Maybe, you met this issue ✨ Empty field values when not renderable Active ? if yes, the workaround must not be in your templates, we need to fix it in the module. It is plan for 2.1 but there may be a way of doing it sooner (with a more targeted scope) in https://www.drupal.org/project/ui_patterns_layout_builder →

Less important: {% set aria_label = aria_label ? aria_label : 'More information' %} can be replaced by {% set aria_label = aria_label|default( 'More information') %}

pagination

UI Patterns Settings 2.2-alpha1 → is released, so we can start moving items from slots (fields) to props (settings). However, for this specific component, it will be complicated (and we will need preprocess hooks for pager and mini pager). I can provide help if necessary.

Less important: {% if item.attributes is not defined %} can be replaced by {% if item.attributes %} to also catch empty attributes or value resolvable to FALSE, because strict_variables is FALSE in Drupal (by design, it is a great feature ;) )

social_link

I don't see this modifier class in documentation and upstream CSS:
.addClass('usa-social-link--' ~ name|lower)}

Also, if name is an enumeration (facebook, twitter...) it would be nice to have a select setting instead od a textfield.

Again: combo_box

options field is not a slot, according to its preview which is not a renderable but a mapping:

<code>
        - text: "Apple"
        - text: "Apricot"
        - text: "Banana Smoothie"

Addressed feedback, added a section to the issue summary about open issues.

For the pagination opened 📌 Update pagination patter Active

options field is not a slot, according to its preview which is not a renderable but a mapping:

- text: "Apple"
- text: "Apricot"
- text: "Banana Smoothie"

Should this be changed to a select? Would have to update the select pattern too maybe?

combo_box

Should this be changed to a select? Would have to update the select pattern too maybe?

The issue is about select component, because combo_box is only using it.

Maybe we just don't need a select component if is purpose is only to add a 'usa-select' class. Overriding https://api.drupal.org/api/drupal/core!modules!system!templates!select.h... is enough.

Also, in combo_box what is the purpose of this code? (honest question)

  'required': required ?: required,
  'disabled': disabled ?: disabled

side_navigation

items field can now be converted to links setting type because UI Patterns Settings 2.2-alpha1 → is released. once those items converted, you can remove the manipulations of the URL objects (getOption, isRouted...).

character_count

attributes object is not used in:

<div class="usa-character-count" data-maxlength="{{ max_length }}">

Again: collection_item

Testing the render array structure in template may be harmful:

{% if meta_items_list['#sources'] is defined %} 
...
{% if meta_items_list['#type'] is defined %}

Normally, slots content are opaque.

step_indicator

Same issue, items is a slot (field) with a sequence of step_indicator_item components, but we have this:

    {% for item in items %}
      {{ item }}
      {% if item['#completed'] == true %}
        {% set completed = completed + 1 %}
      {% endif %}
    {% endfor %}

We need to find a way of keeping the slot opaque.

Quick proposal: Removing step_indicator_item component, having 2 slots in step_indicator:

• completed_items: a sequence of renderables (strings are expected)
• remaining_items: a sequence of renderables (strings are expected)

So, you can loop and count them easily. And also add usa-step-indicator__segment--current class to the last item of completed_items (something missing from current implementation)

summary_box

The content slot preview has this markup:

<ul class="usa-list">
<li>If you are under a winter storm warning, <a class="usa-summary-box__link" href="">find shelter</a> right away.</li>
<li>Sign up for <a class="usa-summary-box__link" href="">your community’s warning system</a>.</li>
<li>Learn the signs of, and basic treatments for, <a class="usa-summary-box__link" href="">frostbite</a> and <a class="usa-summary-box__link" href="">hypothermia</a>.</li>
</ul>'

Because there are usa-summary-box__link classes in this markup, we can guess it belong to the template.

So, what about a list slot with this preview value (non tested proposal):

- - plain_text: If you are under a winter storm warning,
   - type: html_tag
     tag: a
     value: find shelter
     attributes:
        href: ""
   - plain_text: right away.
- - plain_text: Sign up for 
   - type: html_tag
     tag: a
     value: your community’s warning system
     attributes:
        href: ""
- - plain_text: Learn the signs of, and basic treatments for,
   - type: html_tag
     tag: a
     value: frostbite
     attributes:
        href: ""
   - plain_text: and
   - type: html_tag
     tag: a
     value: hypothermia
     attributes:
        href: ""

And then, in the template, something like that (non tested proposal):

    <div class="usa-summary-box__text">
      {{ content }}
     {% if list %}
        <ul class="usa-list">
        {% for item in list %}
          <li>{{ item|add_class("usa-summary-box__link") }}</li>
       {% endfor%}
       </ul>
     {% endif %}
    </div>

Some components doesn't have preview for slots (fields) and props (settings)

I guess, it is done in purpose for the ones which are "sub components":

• accordion_item.ui_patterns.yml
• process_list_item.ui_patterns.yml
• step_indicator_item.ui_patterns.yml
• collection_item_meta_tags.ui_patterns.yml
• collection_item_meta.ui_patterns.yml
• icon_list_item.ui_patterns.yml
• accordion_item.ui_patterns.yml

sub-components will be normal components with an additional "parent" property, which is only altering the library pages:

• They don't have their own preview alone (because most of the time the preview is bad with out the parent component)
• They don't have their own library page: they are displayed in the parent library page

Sub-components are not implemented yet: ✨ [2.1.x] UI Patterns Library: add sub-components Postponed So, in my humble opinion, it is nice to have preview data for them.

What do you think? Do we add this preview data? Or do we speed up the implementation of sub-components in UI Patterns 1.x?

list

This field is not a proper slot because it is a sequence of mapping where one of the value is a renderable:

    rows:
      type: "render"
      label: "Items"
      description: "An array of render arrays."
      preview:
        - content: "Lorem ipsum"
        - content: "Phasellus iaculis"
        - content: "Nulla volutpat"

In Twig:

{% for row in rows %}
  <li{{ row.attributes }}>
    {{- row.content -}}
  </li>
{% endfor %}

So, it will not be easily "site buildable": it will not fit as a layout or as view rows for example.

Do we really need row.attributes here? If not, let's remove it and make rows a simple sequence of renderables.
If yes, I didn't find a proposal yet.

header

this component is missing: https://designsystem.digital.gov/components/header/

I will propose an implementation in 📌 Add landing page example Active MR

hero

Missing from https://designsystem.digital.gov/components/ but expected by https://designsystem.digital.gov/templates/landing-page/ (why? that's weird...)

<section class="usa-hero" aria-label="Introduction">
  <div class="grid-container">
    <div class="usa-hero__callout">
      <h1 class="usa-hero__heading">
        <span class="usa-hero__heading--alt">Hero callout:</span>Bring attention to a project priority
      </h1>
      <p>
        Support the callout with some short explanatory text. You don’t need more than a couple of sentences.
      </p>
      <a class="usa-button" href="">Call to action</a>
    </div>
  </div>
</section>

footer

Missing component.

Described in https://designsystem.digital.gov/components/footer/

3 variants: medium (default), big, slim
5 slots: logo, logo_heading, social_links, contact_heading, contact_info
1 prop: menu (setting type: links)

When it is done, it will be possible to replace the hardcoded markup from in https://git.drupalcode.org/project/ui_suite_uswds/-/blob/3.0.x/templates... by a call to the component.

combo_box

- Updated

input_prefix_suffix

- Removed the field to go inline with how I did other form patterns

side_navigation

- WIP have to research the new way more.

character_count

- Added attributes

collection_item

- Removed all "is defined" from the patterns. I may want to rethink collections in the follow up I opened.

step_indicator

- Going to think on this one som

summary_box

- updated but didn't use plain_text key.

Comment #43
I found that some of the sub components didn't look right or useful without the wrapper classes, so would vote ✨ [2.1.x] UI Patterns Library: add sub-components Postponed

list

- deleted pattern as really wasn't needed.

header, nav, logo, footer

- So I thought about the header and footer options but instead just moved them out to separate twig templates base/includes. Was that a bad idea?

Just pushed some cleanup of the collection patterns and update breadcrumbs to links (I think)

side_navigation

Just a little tip about macros.

There is currently some Twig 2.x code in the template:

{% import _self as menus %}
{{ menus.menu_links(items, 0, 0, attributes) }}
{% macro menu_links(items, menu_level, parent, attributes) %}
  {% import _self as menus %}
  ...
  {{ menus.menu_links(item.below, menu_level + 1, duplicate_parent, attributes) }}
  ...
{% endmacro %}

With Twig 3.x, we do like that:

{{ _self.menu_links(items, 0, 0, attributes) }}
{% macro menu_links(itemmenuss, menu_level, parent, attributes) %}
  ...
  {{ _self.menu_links(item.below, menu_level + 1, duplicate_parent, attributes) }}
  ...
{% endmacro %}

Source: https://twig.symfony.com/doc/3.x/tags/macro.html

step_indicator

Little syntax error:

attributes.addClass('usa-step-indicator--' ~ variant)|replace({'_': '-'})

must be replaced by:

attributes.addClass('usa-step-indicator--' ~ variant|replace({'_': '-'}))

Addressed #51 and #53

language_selector

language_label is not defined

template is printing item.label instead of the expected item.title

item.langcode must be item.attributes.lang.

So, instead of :
<span lang="{{ item.langcode }}" xml:lang="{{ item.langcode }}">{{ item.label }}</span>

You can do:

{% set attributes = attributes.lang ? attributes.setattribute("xml:lang",  attributes.lang ) : attributes %}
<span {{ attributes }}>{{ item.label }}</span>

Just pushed fixes for #55

Oops meant to put back in review.

Sorry, in my previous message, i wrote attributes.setattribute() instead of attributes.setAttribute()
I forgot to tell you my proposal was untested code.

In https://git.drupalcode.org/project/ui_suite_uswds/-/blob/3.0.x/templates... we have both single_message and singe_message:

       {% for single_message in messages %}
          <li class="usa-alert__text">{{ singe_message }}</li>
        {% endfor %}

Fixed #59

Think this can be marked fix, seems like most issues have been around the new components and trying to get logic out of the patterns.

Automatically closed - issue fixed for 2 weeks with no activity.