Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add checks for context attributes #5

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Add checks for context attributes #5

merged 1 commit into from
Jun 7, 2024

Conversation

codechefmarc
Copy link
Collaborator

@codechefmarc codechefmarc commented May 24, 2024

Summary

Note: This was already approved and merged in emulsify_twig as part of PR-61

The changes didn't make their way into emulsify_tools.


The fix includes a check of $context['attributes'] to see if it exists and if not, create a new Attribute(). If it did exist in the first place, a secondary check makes sure that if it's an array, to create an Attribute object from that array.

This fix is based on issues reported on the Drupal.org issue queue:

Fixes

  • Checks $context['attributes'] to see if it exists and if not, create a new Attribute().
  • If $context['attributes'] did exist in the first place, a secondary check makes sure that if it's an array, to create an Attribute object from that array.

Explain the motivation for making this change. What existing problem does the pull request solve?

Sometimes the $context variable does not supply attributes as per https://www.drupal.org/project/emulsify_twig/issues/3160391

Because of this, there are times when it was assumed to be an Attribute object and therefore would throw errors as per https://www.drupal.org/project/emulsify_twig/issues/3210140

Finally, there could be times when the $context['attributes'] is an array. If so, it should be converted to an Attribute object based on that array as per https://www.drupal.org/project/emulsify_twig/issues/3302662

Documentation update (required)

None - this is an internal change only.

How to review this pull request

function MY_THEME_preprocess_page(&$variables) {
  $markup = twig_render_template(\Drupal::service('extension.list.theme')->getPath('MY_THEME') . '/templates/content/node.html.twig', ['content' => 'test content']);
}

The output above doesn't necessarily need to be actually rendered, just this call did produce the error prior to this fix.

  • Other ways of testing this could include testing content using templates with bem and add_attributes in use, inside views, views with Ajax pagers, and search results pages - essentially places that aren't "typical" view modes.

Closing issues

Closes Drupal.org issues listed above.

Copy link
Collaborator

@callinmullaney callinmullaney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codechefmarc Additional $context checks look good to me. Feel free to merge and cut a release to d.o.

@codechefmarc codechefmarc merged commit 92b5643 into main Jun 7, 2024
1 check passed
@codechefmarc codechefmarc deleted the attribute-fix branch June 7, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants