As a follow-up of #3413130: SDC components: Fix JSON schema usage, here are some feedbacks done with https://www.drupal.org/project/sdc_devel which is providing a drush command and a report page to audit SDC components.

Unused variables

Every slot or prop from the component definition must be used in templates.

Default attributes object is not always defined in the YAML, but always injected in template and expected for alteration (adding contextual accessibility attributes, adding data for site building tools, style overrides of the component...).

 $ vendor/bin/drush sdcv --install prototype | grep  "Unused variables:"
 [notice] Start validation of prototype...
menu-tabs    Unused variables: attributes
back-to-top  Unused variables: attributes
search-bar   Unused variables: attributes, form_action, form_children 
slideshow    Unused variables: index_id, title 
icons        Unused variables: attributes

Unknown variables

Every slot or prop used in the template must be found in the component definition (except the ones automatically injected: attributes, componentMetadata...)

$ vendor/bin/drush sdcv --install prototype | grep  "Unknown variable:"
 [notice] Start validation of prototype...
search-bar        Unknown variable: `button_text`.      12     {{ button_text|default('Expand Search') }}                                              
search-bar        Unknown variable: `button_text`.      12     {{ button_text|default('Expand Search') }}                                              
search-bar        Unknown variable: `content`.          16     {{content}}                                                                             
slideshow         Unknown variable: `options`.          26     {% set data_splide = options|default({})|merge(default_options) %}                      
slideshow         Unknown variable: `options`.          26     {% set data_splide = options|default({})|merge(default_options) %}                      
slideshow         Unknown variable: `heading`.          30     <h2 id="{{label_id}}" class="c-slideshow__heading visually-hidden">{{                   
slideshow         Unknown variable: `heading`.          30     <h2 id="{{label_id}}" class="c-slideshow__heading visually-hidden">{{                   
slideshow         Unknown variable: `slides`.           34     {% for slide in slides %}                                                               
menu              Unknown variable: `classes`.          2      'class': ['c-menu']|merge(classes|default([])),                                         
menu              Unknown variable: `classes`.          2      'class': ['c-menu']|merge(classes|default([])),                                         
menu              Unknown variable: `menu_name`.        19     {{ menus.menu_links(items, attributes.addClass('c-menu__list'), 0, menu_name) }}        
menu              Unknown variable: `classes`.          25     <li {{ item.attributes.addClass(classes) }}>                                            
drupal-messages   Unknown variable: `status_headings`.  10     <div role='contentinfo' aria-label="{{ status_headings[type] }}" {{                     
drupal-messages   Unknown variable: `status_headings`.  15     {% if status_headings[type] %}                                                          
drupal-messages   Unknown variable: `status_headings`.  16     <h2 class='visually-hidden'>{{ status_headings[type] }}</h2>                            
sidebar           Unknown variable: `class`.            2      <aside{{attributes.addClass(class)}} role='complementary'>

Incomplete prop definitions

Empty objects are not enough to be understood and processed by display builders (UI Patterns 2, the upcoming Experience Builder...):

$ vendor/bin/drush sdcv --install prototype  | grep "Empty array"
 [notice] Start validation of prototype...
teaser            Empty array.   23     media:                                                                                  
pager             Empty array.   11     items:                                                                                  
pager             Empty array.   14     ellipses:                                                                               
tabs-content      Empty array.   14     items:

Issue fork prototype-3502491

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

pixelwhip made their first commit to this issue’s fork.

jldust’s picture

jldust commented
Assigned: Unassigned » jldust
Priority: Normal » Major

grgcrlsn321 made their first commit to this issue’s fork.

grgcrlsn321 opened merge request !215

grgcrlsn321’s picture

grgcrlsn321 commented
Status: Active » Needs work

I've added props and updated props that needed fixed.

YAML fixes — wrong prop names replaced:
button.component.yml — replaced icon with icon_prefix and icon_suffix
slideshow.component.yml — replaced title with heading, options, and slides
search-bar.component.yml — replaced form_action and form_children with prop button_text and slot content
breadcrumbs.component.yml — renamed slot breadcrumbs_block → items_block, added prop id

YAML additions — missing props added:
menu.component.yml — added menu_name and classes
sidebar.component.yml — added class

Twig fixes — dead variables removed:
pager.twig — removed {% set is_current %} (never output; aria-current already written inline)
slideshow.twig — removed {% set index_id %} (never referenced in template output)

Twig workaround — macro-only component:
icons.twig — added {% if attributes %}{% endif %} to satisfy the validator for the auto-injected attributes variable in a macro-only file

grgcrlsn321’s picture

grgcrlsn321 commented
Version: 5.2.1-alpha5 » 5.1.x-dev
Assigned: jldust » grgcrlsn321
grgcrlsn321’s picture

grgcrlsn321 commented
Assigned: grgcrlsn321 » Unassigned
Status: Needs work » Needs review

Fixes:

  • teaser:media was using wrong type array when twig template is a single value not a loop for an array. Changed type to string.
  • tabs-content:items is type array, but there were no items in component. Added items necessary to match schema with template.
  • pager:items and pager:ellipses appear to have items that are not empty for array.

This fixes the remainder of the errors on this issue. Setting to needs review.

grgcrlsn321’s picture

grgcrlsn321 commented

I had a workaround for the icons component, but I don't think it's that great and could cause more confusion. There aren't any attributes for that component so creating an if statement is not very helpful for it's use case. That means, there is still a warning that we should ignore:

drush sdcv --install prototype | grep  "Unused variables:"
v18.20.8 is already installed.
 [notice] Start validation of prototype...
 [warning] Found 13 problems in prototype.
  prototype:icons             Error      Unused variables: attributes                                                   Twig   -
pdureau’s picture

pdureau commented

There aren't any attributes for that component

Maybe because this component is not... a component ;) Why not using the Core Icon API to manage this?

grgcrlsn321’s picture

grgcrlsn321 commented
Assigned: Unassigned » grgcrlsn321
grgcrlsn321’s picture

grgcrlsn321 commented

okay, I went ahead and moved icons to a macros directory to not include it with the components. This removes the warnings and defines the macro more clearly. We can't fully support the Core Icon API with Drupal sites using 10.3. Maybe in a future upgrade and we drop 10.3 support we can look at using Core Icon API. Setting to needs review.

jldust’s picture

jldust commented

I removed the macro in favor of the updated icons component, going to need additional testing to confirm this is the direction we want to go for now. I think we should create another issue for reviewing the Core Icon API option.

pdureau’s picture

pdureau commented

We can't fully support the Core Icon API with Drupal sites using 10.3. Maybe in a future upgrade and we drop 10.3 support we can look at using Core Icon API.

UI Icons module has a copy of the Drupal 11.x Icon API in its 1.0.x branch, to be used with Drupal 10.x

jldust’s picture

jldust commented
Assigned: grgcrlsn321 » jldust
Status: Needs review » Reviewed & tested by the community

Noting that I've created a dedicated issue for us to look into Drupal Icons API and how to support that.

jldust’s picture

jldust commented

This has been merged in and will be included in the next tagged release.

jldust’s picture

jldust commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

jldust’s picture

jldust commented
Status: Fixed » Closed (fixed)