1
\$\begingroup\$

I used the Bootstrap v4.0.0 accordion to display my business services. Further, I created a custom post type "service" for my services (Each service is a post).

To display my services dynamically and not hardcode them, I implemented a for-loop within the WordPress while-loop which uses an enumeration to provide the number of the collapsible.

I don't like my solution, especially the enumeration - Is there a better way to solve this? If you see any other improvement, I would be glad to hear also.

See my code below:

<div class="wv-archive-container wv-boxed-wrapper">
    <div class="container">
        <div class="row">
            <div class="col-sm">
                <h1 id="wv-archive-header-h1">All Services</h1>
                <div id="wv-accordion" class="wv-accordion">
                    <div class="card mb-0">

                        <?php $wv_services = new WP_Query( array(
                            'posts_per_page' => -1,
                            'post_type'      => 'service',
                            'orderby'        => 'date',
                            'order'          => 'ASC'
                        ) );

                        while ( $wv_services->have_posts() ) {

                            $enum_of_collapsible_str_numbers = [
                                "One",
                                "Two",
                                "Three",
                                "Four",
                                "Five",
                                "Six",
                                "Seven",
                                "Eight",
                                "Nine",
                                "Ten",
                                "Eleven",
                                "Twelve",
                                "Thirtenn",
                                "Fourteen",
                                "Fivteen",
                                "Sixteen",
                                "Seventeen",
                                "Eighteen",
                                "Nineteen",
                                "Twenty"
                            ];
                            $total_number_of_service_posts = wp_count_posts('service')->publish;
                            for ($collapsible_str_number = 0; $collapsible_str_number < $total_number_of_service_posts; $collapsible_str_number++) {

                                $wv_services->the_post(); ?>

                                <div class="card-header collapsed" style="cursor:pointer;" data-toggle="collapse"
                                     href="#collapse<?php echo $enum_of_collapsible_str_numbers[$collapsible_str_number] ?>" rel=”nofollow”>
                                    <a class="card-title"><h2><?php the_title(); ?></h2></a>
                                </div>
                                <div id="collapse<?php echo $enum_of_collapsible_str_numbers[$collapsible_str_number] ?>" class="card-body collapse"
                                     data-parent="#accordion">
                                    <p><?php the_content(); ?></p>
                                </div>

                            <?php }
                        }
                        wp_reset_postdata();
                        ?>

                    </div>
                </div>
            </div>
        </div>
    </div>
</div>
\$\endgroup\$
1
\$\begingroup\$

Not sure what you are trying to achieve, but you can use numerical values for enum_of_collapsible_numbers and you can generate them like this

$enum_of_collapsible_numbers = range(1, 20);
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I personally do not prefer the mixing of data collection with data printing, but I understand that this is common on simple Wordpress pages (I am not a Wordpress developer).

id values do not need to be purely comprised of letters to be valid, so I would use a counter and scrap the idea of having a limited-length lookup array of words.

Using printf() will help to clean up your code by removing string concatenation/interpolation; it also allows you to reuse the same variable in the same expression. Generally, I don't enjoy trying to read a bunch of <?php ... ?> declarations bouncing in and out of php in script -- it just feels too noisy.

Move your cursor:pointer; inline declaration to an external stylesheet inside .collapsed{} to reduce some of the eye strain on this script.

Consider declaring your <div> attributes on multiple lines to prevent excessively long lines of code.

Do not use (multibyte) curly quotes to encapsulate attribute values.

I think I recommend writing <a> inside of <h2> instead of the inverse.

Untested suggestion:

$wv_services = new WP_Query([
    'posts_per_page' => -1,
    'post_type'      => 'service',
    'orderby'        => 'date',
    'order'          => 'ASC'
]);

while ($wv_services->have_posts()) {
    $post_count = wp_count_posts('service')->publish;
    for ($i = 0; $i < $post_count; ++$i) {
        $wv_services->the_post();
        sprintf('
            <div class="card-header collapsed"
                 data-toggle="collapse"
                 href="#collapse%1$d"
                 rel="nofollow"
            >
                <h2>
                    <a class="card-title">%2$s</a>
                </h2>
            </div>
            <div id="collapse%1$d"
                 class="card-body collapse"
                 data-parent="#accordion"
            >
                <p>%3$s</p>
            </div>',
            $i,             // referenced by %1$d
            the_title(),    // referenced by %2$s
            the_content()   // referenced by %3$s
        );
    }
}
wp_reset_postdata();
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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