Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Removes invalid render key option. #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amykhailova
Copy link

No description provided.

foreach (Element::children($variables) as $key) {
$vars = $variables;
// Remove all not valid render keys.
foreach ($vars as $key => $var) {
Copy link

Choose a reason for hiding this comment

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

I'm disagree with your modifications. The drupal good practice is to use "Element::children". But the implementation in the loop is not correct.

Copy link
Author

Choose a reason for hiding this comment

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

It's ok to disagree :). However without it invalid render keys were there and the module wasn't working, returning a fatal error. With what I did the module works and doesn't return any errors. There are many different ways of accomplishing tasks in web and it's not necessarily wrong to do things differently. It's not the foreach loop that matters in this case it's what's happening inside it. Notice how I prepare the variables and only then use Element::children. The foreach I used just cleans up the array.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants