zen_get_products_quantity_min_units_display

**yesaul** · 25 Aug 2021, 04:13 PM

lines535...549 of includes/functions/function_prices.php:

Code:

    // quantity max
    $check_max = zen_get_products_quantity_order_max($product_id);

    if ($check_max != 0) {
      $the_min_units .= '<span class="qmax">';
      if ($include_break == true) {
        $the_min_units .= ($the_min_units != '' ? '<br />' : '') . PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
      } else {
        $the_min_units .= ($the_min_units != '' ? '&nbsp;&nbsp;' : '') . PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
      }
      $the_min_units .= '</span>';
    }

    return $the_min_units;
  }

AS '<span class="qmax">' string is being added (assigned) to $the_min_units variable, there is no sence to check if it is empty later.

My suggestion:

Code:

    // quantity max
    $check_max = zen_get_products_quantity_order_max($product_id);

    if ($check_max != 0) {

      if ($include_break == true) {
        $the_min_units .= ($the_min_units != '' ? '<br />' : '') . '<span class="qmax">' . PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
      } else {
        $the_min_units .= ($the_min_units != '' ? '&nbsp;&nbsp;' : '') . '<span class="qmax">' . PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
      }
      $the_min_units .= '</span>';
    }

**mc12345678** · 25 Aug 2021, 05:02 PM

Nevermind, found the code. Scuse me.. :)

**carlwhat** · 25 Aug 2021, 10:29 PM

Originally Posted by yesaul

lines535...549 of includes/functions/function_prices.php:
...

valid point.

i would suggest having code discussions and potentially doing a PR over on github. this section of code looks to have some changes in v158:

https://github.com/zencart/zencart/b....php#L559-L616

and your "bug" seems to have been addressed. please correct me if i am wrong.

this $include_break test seems to be done a few times as well. i see other refactor potential, as in:

PHP Code:


$breakElement = '&nbsp;&nbsp;'

if ($include_break) {

    $breakElement = '<br>';

}

and then we can reduce the 3 conditionals to 1, and just include that $breakElement var as needed. which i personally thinks improves readability.

best.

**mc12345678** · 26 Aug 2021, 03:04 PM

When looking at the ZC 1.5.8 version of this area, I question the importance of *always* forcing the additional character(s) associated with $the_min_units and/or why the span's class must *always* encompass that information? E.g. why not place the span information after the additional assignment of $the_min_units instead of incorporating the span into the following text.

Instead of:

Code:

    if ($check_max > 0) {
        $the_min_units .= '<span class="qmax">';
        if ($include_break == true) {
            $the_min_units .= ($the_min_units != '' ? '<br>' : '');
        } else {
            $the_min_units .= ($the_min_units != '' ? '&nbsp;&nbsp;' : '');
        }
        $the_min_units .= PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
        $the_min_units .= '</span>';
    }

Making it:

Code:

    if ($check_max > 0) {
        if ($include_break == true) {
            $the_min_units .= ($the_min_units != '' ? '<br>' : '');
        } else {
            $the_min_units .= ($the_min_units != '' ? '&nbsp;&nbsp;' : '');
        }
        $the_min_units .= '<span class="qmax">';
        $the_min_units .= PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
        $the_min_units .= '</span>';
    }

Or if incorporating the above "breakElement":

Code:

    if ($check_max > 0) {
        $the_min_units .= ($the_min_units != '' ? $breakElement : '');
        $the_min_units .= '<span class="qmax">';
        $the_min_units .= PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
        $the_min_units .= '</span>';
    }

If the "span" does need to fit around the break (assuming a break like character is needed because of earlier text), then would suggest generating an additional variable to support the check being able to omit inserting the break element when it is "currently" not required:

Code:

    if ($check_max > 0) {
        $span_max_class = '<span class="qmax">';
        $the_min_units .= $span_max_class;
        $the_min_units .= ($the_min_units != $span_max_class ? $breakElement : '');
        $the_min_units .= PRODUCTS_QUANTITY_MAX_TEXT_LISTING . '&nbsp;' . $check_max;
        $the_min_units .= '</span>';
    }

This is suggested because there are those that don't want a break forced for an empty set of previous text either for html validation or look and to not require some sort of additional CSS, javascript/jQuery, or other code to "remove".