Merge pull request #259 from mimmi20/feature-reduce-complexity

Reducing the complexity of view helpers
laminas · Jan 9, 2024 · 1c9ac51 · 1c9ac51
2 parents b3c6575 + a6c287d
commit 1c9ac51
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 107 deletions.
diff --git a/src/View/Helper/AbstractFormDateSelect.php b/src/View/Helper/AbstractFormDateSelect.php
@@ -203,13 +203,13 @@ protected function getMonthsOptions(string $pattern): array
      * NOTE: we don't use a pattern for years, as years written as two digits can lead to hard to
      * read date for users, so we only use four digits years
      *
-     * @return array
+     * @return array<int, string>
      */
     protected function getYearsOptions(int $minYear, int $maxYear): array
     {
         $result = [];
         for ($i = $maxYear; $i >= $minYear; --$i) {
-            $result[$i] = $i;
+            $result[$i] = (string) $i;
         }
 
         return $result;

diff --git a/src/View/Helper/AbstractHelper.php b/src/View/Helper/AbstractHelper.php
@@ -565,7 +565,7 @@ protected function hasAllowedPrefix(string $attribute): bool
      *
      * @internal
      */
-    protected function translateLabel(string|int $label): string|int
+    protected function translateLabel(string $label): string
     {
         return $this->getTranslator()?->translate($label, $this->getTranslatorTextDomain()) ?? $label;
     }

diff --git a/src/View/Helper/FormCollection.php b/src/View/Helper/FormCollection.php
@@ -113,6 +113,7 @@ public function render(ElementInterface $element): string
         $templateMarkup = '';
         $elementHelper  = $this->getElementHelper();
         assert(is_callable($elementHelper));
+
         $fieldsetHelper = $this->getFieldsetHelper();
         assert(is_callable($fieldsetHelper));
 
@@ -128,43 +129,41 @@ public function render(ElementInterface $element): string
             }
         }
 
-        // Every collection is wrapped by a fieldset if needed
-        if ($this->shouldWrap) {
-            $attributes = $element->getAttributes();
-            if (! $this->getDoctypeHelper()->isHtml5()) {
-                unset(
-                    $attributes['name'],
-                    $attributes['disabled'],
-                    $attributes['form']
-                );
-            }
-            $attributesString = $attributes !== [] ? ' ' . $this->createAttributesString($attributes) : '';
+        if (! $this->shouldWrap) {
+            return $markup . $templateMarkup;
+        }
 
-            $label  = $element->getLabel();
-            $legend = '';
+        // Every collection is wrapped by a fieldset if needed
+        $attributes = $element->getAttributes();
+        if (! $this->getDoctypeHelper()->isHtml5()) {
+            unset(
+                $attributes['name'],
+                $attributes['disabled'],
+                $attributes['form']
+            );
+        }
+        $attributesString = $attributes !== [] ? ' ' . $this->createAttributesString($attributes) : '';
 
-            if (! empty($label)) {
-                $label = $this->translateLabel($label);
-                $label = $this->escapeLabel($element, $label);
+        $label  = $element->getLabel();
+        $legend = '';
 
-                $legend = sprintf(
-                    $this->labelWrapper,
-                    $label
-                );
-            }
+        if (! empty($label)) {
+            $label = $this->translateLabel($label);
+            $label = $this->escapeLabel($element, $label);
 
-            $markup = sprintf(
-                $this->wrapper,
-                $markup,
-                $legend,
-                $templateMarkup,
-                $attributesString
+            $legend = sprintf(
+                $this->labelWrapper,
+                $label
             );
-        } else {
-            $markup .= $templateMarkup;
         }
 
-        return $markup;
+        return sprintf(
+            $this->wrapper,
+            $markup,
+            $legend,
+            $templateMarkup,
+            $attributesString
+        );
     }
 
     /**

diff --git a/src/View/Helper/FormRow.php b/src/View/Helper/FormRow.php
@@ -119,14 +119,14 @@ public function render(ElementInterface $element, ?string $labelPosition = null)
         $elementHelper       = $this->getElementHelper();
         $elementErrorsHelper = $this->getElementErrorsHelper();
 
-        $label           = $element->getLabel();
+        $label           = $element->getLabel() ?? '';
         $inputErrorClass = $this->getInputErrorClass();
 
         if ($labelPosition === null) {
             $labelPosition = $this->labelPosition;
         }
 
-        if (isset($label) && '' !== $label) {
+        if ('' !== $label) {
             // Translate the label
             $label = $this->translateLabel($label);
         }
@@ -160,81 +160,76 @@ public function render(ElementInterface $element, ?string $labelPosition = null)
 
         // hidden elements do not need a <label> -https://github.com/zendframework/zf2/issues/5607
         $type = $element->getAttribute('type');
-        if (isset($label) && '' !== $label && $type !== 'hidden') {
-            $labelAttributes = [];
-
-            if ($element instanceof LabelAwareInterface) {
-                $labelAttributes = $element->getLabelAttributes();
-            }
-
-            $label = $this->escapeLabel($element, $label);
-
-            if (empty($labelAttributes)) {
-                $labelAttributes = $this->labelAttributes;
-            }
-
-            // Multicheckbox elements have to be handled differently as the HTML standard does not allow nested
-            // labels. The semantic way is to group them inside a fieldset
-            if (
-                $type === 'multi_checkbox'
-                || $type === 'radio'
-                || $element instanceof MonthSelect
-                || $element instanceof Captcha
-            ) {
-                $markup = sprintf(
-                    '<fieldset><legend>%s</legend>%s</fieldset>',
-                    $label,
-                    $elementString
-                );
-            } else {
-                // Ensure element and label will be separated if element has an `id`-attribute.
-                // If element has label option `always_wrap` it will be nested in any case.
-                if (
-                    $element->hasAttribute('id')
-                    && ($element instanceof LabelAwareInterface && ! $element->getLabelOption('always_wrap'))
-                ) {
-                    $labelOpen  = '';
-                    $labelClose = '';
-                    $label      = $labelHelper->openTag($element) . $label . $labelHelper->closeTag();
-                } else {
-                    $labelOpen  = $labelHelper->openTag($labelAttributes);
-                    $labelClose = $labelHelper->closeTag();
-                }
-
-                if (
-                    $label !== '' && (! $element->hasAttribute('id'))
-                    || ($element instanceof LabelAwareInterface && $element->getLabelOption('always_wrap'))
-                ) {
-                    $label = '<span>' . $label . '</span>';
-                }
-
-                // Button element is a special case, because label is always rendered inside it
-                if ($element instanceof Button) {
-                    $labelOpen = $labelClose = $label = '';
-                }
-
-                if ($element instanceof LabelAwareInterface && $element->getLabelOption('label_position')) {
-                    $labelPosition = $element->getLabelOption('label_position');
-                }
-
-                $markup = match ($labelPosition) {
-                    self::LABEL_PREPEND => $labelOpen . $label . $elementString . $labelClose,
-                    default => $labelOpen . $elementString . $label . $labelClose,
-                };
-            }
-
-            if ($this->renderErrors) {
-                $markup .= $elementErrors;
-            }
+
+        if ($label === '' || $type === 'hidden') {
+            return $elementString . $elementErrors;
+        }
+
+        $labelAttributes = [];
+
+        if ($element instanceof LabelAwareInterface) {
+            $labelAttributes = $element->getLabelAttributes();
+        }
+
+        $label = $this->escapeLabel($element, $label);
+
+        if (empty($labelAttributes)) {
+            $labelAttributes = $this->labelAttributes;
+        }
+
+        // Multicheckbox elements have to be handled differently as the HTML standard does not allow nested
+        // labels. The semantic way is to group them inside a fieldset
+        if (
+            $type === 'multi_checkbox'
+            || $type === 'radio'
+            || $element instanceof MonthSelect
+            || $element instanceof Captcha
+        ) {
+            $markup = sprintf(
+                '<fieldset><legend>%s</legend>%s</fieldset>',
+                $label,
+                $elementString
+            );
+
+            return $markup . $elementErrors;
+        }
+
+        // Ensure element and label will be separated if element has an `id`-attribute.
+        // If element has label option `always_wrap` it will be nested in any case.
+        if (
+            $element->hasAttribute('id')
+            && ($element instanceof LabelAwareInterface && ! $element->getLabelOption('always_wrap'))
+        ) {
+            $labelOpen  = '';
+            $labelClose = '';
+            $label      = $labelHelper->openTag($element) . $label . $labelHelper->closeTag();
         } else {
-            if ($this->renderErrors) {
-                $markup = $elementString . $elementErrors;
-            } else {
-                $markup = $elementString;
-            }
+            $labelOpen  = $labelHelper->openTag($labelAttributes);
+            $labelClose = $labelHelper->closeTag();
+        }
+
+        if (
+            $label !== '' && (! $element->hasAttribute('id'))
+            || ($element instanceof LabelAwareInterface && $element->getLabelOption('always_wrap'))
+        ) {
+            $label = '<span>' . $label . '</span>';
         }
 
-        return $markup;
+        // Button element is a special case, because label is always rendered inside it
+        if ($element instanceof Button) {
+            $labelOpen = $labelClose = $label = '';
+        }
+
+        if ($element instanceof LabelAwareInterface && $element->getLabelOption('label_position')) {
+            $labelPosition = $element->getLabelOption('label_position');
+        }
+
+        $markup = match ($labelPosition) {
+            self::LABEL_PREPEND => $labelOpen . $label . $elementString . $labelClose,
+            default => $labelOpen . $elementString . $label . $labelClose,
+        };
+
+        return $markup . $elementErrors;
     }
 
     /**