Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Hardik_Patel_12 created an issue. See original summary.

Hardik_Patel_12’s picture

Kindly review a patch.

Hardik_Patel_12’s picture

Hardik_Patel_12’s picture

Assigned: Hardik_Patel_12 » Unassigned
Status: Needs work » Needs review
rogerpfaff’s picture

Status: Needs review » Reviewed & tested by the community

Quite simple code change. Tests are working.

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php
@@ -359,9 +359,9 @@ public function label() {
 
-    foreach ($this->getSections() as $delta => $section) {
+    foreach ($this->getSections() as $section) {
       $this->calculatePluginDependencies($section->getLayout());
-      foreach ($section->getComponents() as $uuid => $component) {
+      foreach ($section->getComponents() as $component) {
         $this->calculatePluginDependencies($component->getPlugin());
       }

People often specify the key in foreaches for readability, so let's remove these hunks from the patch and open a coding standards issue if we want to standardise one way or the other.

rogerpfaff’s picture

The use of the key is not in general defined in the coding standards. The example in the PEAR coding standards shows it with key but all over drupal it is used in both ways. Seems like it is used where the key is needed and if not it is left out. Also in this case the key is not something special needed anywhere. I would say it's ok to keep the hunks.

Rangaswini’s picture

Assigned: Unassigned » Rangaswini
pratik_kamble’s picture

Assigned: Rangaswini » pratik_kamble
pratik_kamble’s picture

Rerolled patch to keep the key in foreach.

pratik_kamble’s picture

Assigned: pratik_kamble » Unassigned
Status: Needs work » Needs review
nishantghetiya’s picture

Assigned: Unassigned » nishantghetiya
nishantghetiya’s picture

Assigned: nishantghetiya » Unassigned
Status: Needs review » Reviewed & tested by the community

@pratik_kamble Hello,

Your patch is works well. Thanks for your contribution.

  • catch committed 6ef456a on 9.0.x
    Issue #3108252 by pratik_kamble, Hardik_Patel_12, rogerpfaff,...

  • catch committed 97f7385 on 8.9.x
    Issue #3108252 by pratik_kamble, Hardik_Patel_12, rogerpfaff,...
catch’s picture

Version: 9.0.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 6ef456a and pushed to 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

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