0

I have written some hacky code that breaks the DRY principle ten times to get ten slider images on the home page of a site I'm building. I have to repeat this code 10 times and it's really ugly!

<?php if (get_post_meta($post->ID, '1-carousel-image')==true): ?>
    <div class="item">
      <?php if (get_post_meta($post->ID, '1-carousel-title')==true): ?>
      <div class="display-block">
        <div class="text-block slider">
          <h3 class="slider-title"><?php echo get_post_meta($post->ID, '1-carousel-title', true); ?></h3>
          <p><?php echo get_post_meta($post->ID, '1-carousel-text', true); ?></p>
          <?php if (get_post_meta($post->ID, '1-carousel-button-text')==true): ?>
          <a href="<?php echo get_site_url(); ?><?php echo get_post_meta($post->ID, '1-carousel-button-link', true); ?>" class="button primary"><?php echo get_post_meta($post->ID, '1-carousel-button-text', true); ?></a>
          <?php else: ?>
          <?php /* Show no button if text is missing */ ?>
          <?php endif; ?>
        </div>
      </div>
      <?php else: ?>
      <?php /* Show no text if text is missing */ ?>
      <?php endif; ?>
      <img class="mobile-hide" src="<?php echo get_post_meta($post->ID, '1-carousel-image', true); ?>" alt="<?php echo get_post_meta($post->ID, '1-carousel-image-alt', true); ?>">
      <img class="mobile-show" src="<?php echo get_post_meta($post->ID, '1-carousel-image-mobile', true); ?>" alt="<?php echo get_post_meta($post->ID, '1-carousel-image-alt', true); ?>">
    </div>
    <?php else: ?>
    <?php /* Show nothing if video custom post type is missing */ ?>
    <?php endif; ?>

I know I need a for loop but haven't been able to write it correctly. How do I do it with the above code and custom post type?

Here's the steps I want:

  1. Have a for loop that starts at 1 and goes up using a variable and stops when it runs out of images
  2. The loop must check if there's a title and then the title block shows
  3. Once the loop finds the first image then it checks for other data and adds those if available
  4. The loop repeats until no more images are available; 1-carousel-image then checks 2-carousel-image, and so on.

I want to create a variable that is x-carousel-image, x-carousel-title, etc. that keeps increasing by one till no more images are available. How do I write this in php?

1 Answer 1

1

Sure thing, couple of pointers:

  1. No need for an empty else - just go straight to the endif
  2. == true is a redundant condition. if ( $var ) and if ( $var == true ) are the same. Note that $var === true is different though (read up on comparison operators)
  3. Use site_url( $path ) instead of get_site_url() . $path - this will ensure the URL is properly concatenated (if $path had no preceding slash you'd end up with example.commypath)
  4. Sidenote: might want to re-think your mobile images technique - regardless of your CSS/JS, any device is going to request both images

And save yourself even more typing by assigning values within the condition. For example:

// This...
if ( get_post_meta( $id, $field ) )
    echo get_post_meta( $id, $field );

// Less function overhead, neater
$value = get_post_meta( $id, $field );
if ( $value )
    echo $value;

// Even neater
if ( $value = get_post_meta( $id, $field ) )
    echo $value;

The result:

<?php

for ( $i = 1; $i <= 10; $i++ ) {
    if ( $image = get_post_meta( $post->ID, "{$i}-carousel-image", true ) ) : ?>

        <div class="item">
            <?php if ( $title = get_post_meta( $post->ID, "{$i}-carousel-title", true ) ) : ?>
                <div class="display-block">
                    <div class="text-block slider">
                        <h3 class="slider-title"><?php echo $title ?></h3>
                        <p><?php echo get_post_meta( $post->ID, "{$i}-carousel-text", true ) ?></p>
                        <?php if ( $text = get_post_meta( $post->ID, "{$i}-carousel-button-text", true ) ) : ?>
                            <a href="<?php echo esc_url( site_url( get_post_meta( $post->ID, "{$i}-carousel-button-link", true ) ) ) ?>" class="button primary"><?php echo $text ?></a>
                        <?php endif ?>
                    </div>
                </div>

            <?php endif ?>
            <img class="mobile-hide" src="<?php echo esc_url( $image ) ?>" alt="<?php echo esc_attr( get_post_meta( $post->ID, "{$i}-carousel-image-alt", true ) ) ?>" />
            <img class="mobile-show" src="<?php echo esc_url( get_post_meta( $post->ID, "{$i}-carousel-image-mobile", true ) ) ?>" alt="<?php echo esc_attr( get_post_meta( $post->ID, "{$i}-carousel-image-alt", true ) ) ?>" />
        </div>

    <?php
    endif;
}
3
  • OMG thanks for this it's awesome! How can I utilise wp built in responsive images to replace what I'm doing now? Aug 26, 2016 at 9:47
  • If the image is an upload in the media gallery, store the ID for the image (instead of the URL) for {$i}-carousel-image and then use echo wp_get_attachment_image( $image, 'full' ), which will automatically output the srcset attribute. Aug 26, 2016 at 9:54
  • Quick update I escaped the button url link and fixed the closing tag for the for loop: Oct 5, 2016 at 22:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.