22
\$\begingroup\$

This is a simple list of items in a list, which allows the user to dynamically generate that list of events. Then a controller action does the work of serializing that into the database.

The issue is that there's the PHP-generated HTML segment, and there's a separate JavaScript segment to do the additions (for when someone presses the "add new page" button). This is not only duplicated; when it's done inline in the JavaScript it's extremely ugly to look at (look at the length of that line!).

Is there a better way of doing this?

<?php
/** @var $this Zend_View */

$this->headLink()->appendStylesheet($this->baseUrl('css/redmond/jquery-ui-1.8.5.custom.css'));

$this->headScript()->appendFile($this->baseUrl('js/jquery.js'));
$this->headScript()->appendFile($this->baseUrl('js/jquery-ui-1.8.5.custom.min.js'));

$this->headScript()->captureStart(); ?>
//<script language="text/javascript">
    function CreateDateboxes(jqObject) {
        jqObject.datepicker({
            dateFormat: 'yy-mm-dd',
            showOn: 'button',
            changeYear: true,
            changeMonth: true
        });
    }

    function RemoveParent() {
        $(this).parent().parent().remove();
    }

    function AddNewEvent() {
        var today, html, temp, datestring;
        today = new Date();
        datestring = (today.getYear()+1900) + '-' + (today.getMonth()+1) + '-' + today.getDate();
        html = '<li class="ui-content">\n    <div style="float: left; width: 200px;">\n        <span class="ui-icon ui-icon-trash ui-button ui-state-active" style="float: left; margin:3px;"></span>\n        <input name="dates[]" class="datebox" style="width: 120px;" type="text" value="' + datestring + '" />\n    </div>\n    <div style="padding-left: 200px;">\n        <input name="contents[]" type="text" style="width: 100%;" />\n    </div>\n</li>';
        $(this).after(html);
        temp = $(this).next();
        CreateDateboxes($('.datebox', temp));
        $('.ui-icon-trash', temp).click(RemoveParent);
    }

    $(function() {
        CreateDateboxes($('.datebox'));
        $('.ui-icon-trash').click(RemoveParent);
        $('.ui-icon-plus').parent().click(AddNewEvent);
    });
//</script>
<?php $this->headScript()->captureEnd(); ?>
<div class="story">
    <form action="<?= $this->url(array('controller' => 'admin', 'action' => 'applyEvents')) ?>" method="post">
        <ul style="list-style: none; margin-bottom: 1em; padding: 0; width: 100%;">
            <li class="ui-button ui-state-default" style="width: 100%; margin-bottom: 5px;"><span class="ui-icon ui-icon-plus" style="float: left;"></span>
                Add a new Event</li>
            <? foreach ($this->events as $event) { ?>
                <li class="ui-content">
                    <div style="float: left; width: 200px;">
                    <span class="ui-icon ui-icon-trash ui-button ui-state-active" style="float: left; margin:3px;"></span>
                    <input name="dates[]" class="datebox" style="width: 120px;" type="text" value="<?= $event->GetDate()->format('Y-m-d') ?>" />
                    </div>
                    <div style="padding-left: 200px;">
                        <input name="contents[]" type="text" value="<?= $event->GetMessage() ?>" style="width: 100%;" />
                    </div>
                </li>
            <? } ?>
        </ul>
        <input type="submit" name="submit" value="Apply" />
        <input type="submit" name="submit" value="Cancel" />
    </form>
</div>
\$\endgroup\$
  • \$\begingroup\$ Use something like Mustache, which is a templating framework that has renderers both on the client-side and the server side, which allows you to avoid code duplication. \$\endgroup\$ – glebm Jan 31 '11 at 21:27
  • \$\begingroup\$ Templating is definitely the answer. Another option is beebole.com/pure for client side templating. With templating you can write your html in one place, then add it to a variable in javascript, then append the variables html into a node applying the template. No duplicate html, no ajax. \$\endgroup\$ – Jason McCarrell Feb 1 '11 at 1:40
  • \$\begingroup\$ I ran into a similar problem just a week ago and I got tired of duplicating really fast, too. I just created one JavaScript function to do the printing of data to the page, and made my PHP code to print JavaScript calls to that function. The code became much cleaner that way. \$\endgroup\$ – Rogach Feb 10 '11 at 14:58
8
\$\begingroup\$

I'm assuming from looking the code you're using Zend Framework. I've solved similar issues in Zend and Symfony using php partials.

You could create a partial _list_item.phtml

<li class="ui-content">
    <div style="float: left; width: 200px;">
        <span class="ui-icon ui-icon-trash ui-button ui-state-active" style="float: left; margin:3px;"></span>
        <input name="dates[]" class="datebox" style="width: 120px;" type="text" value="<?php echo $date ?>" />
    </div>
    <div style="padding-left: 200px;">
        <input name="contents[]" type="text" value="<?php echo $message ?>" style="width: 100%;" />
    </div>
</li>

Then in your view:

<?php foreach ($this->events as $event): ?>
    <?php echo $this->partial('list_item.phtml', array(
        'date' => $event->GetDate()->format('Y-m-d'),
        'message' => $event->GetMessage(),
    )); ?>
<?php endforeach; ?>

Then for your javascript:

function AddNewEvent()
{
    var today, html, temp;
    // call str_replace because javascript doesn't like new lines in strings
    html = '<?php echo str_replace(PHP_EOL, "\n", $this->partial('list_item.phtml', array(
        'date' => $event->GetDate()->format('Y-m-d'),
        'message' => '',
    ))); ?>';
    $(this).after(html);
    temp = $(this).next();
    CreateDateboxes($('.datebox', temp));
    $('.ui-icon-trash', temp).click(RemoveParent);
}

Viola! No template duplication because the affected code is in a partial. No excess javscript templating libraries, although if this kind of thing is prevalent in your application I'd suggest some refactoring and implementing such a templating library.

This approach will have the least impact on your work flow, little to no learning curve, and a short investment in development time for big decrease duplication (bosses love hearing that kind of thing).

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 Slick! (But if you're doing this in production you should probably use json_encode instead of str_replace :) \$\endgroup\$ – Billy ONeal Feb 10 '11 at 16:03
  • \$\begingroup\$ This also didn't set the date on the client as the preivous code had done. I ended up using something like this except I replaced the javascript with pastebin.com/MmFcrbHu \$\endgroup\$ – Billy ONeal Feb 13 '11 at 0:53
  • \$\begingroup\$ It should have print the date. The issue was a type in my partial _list_item.phtml. Just replace the echo $message with echo $date. My mistake. I've update the code above. \$\endgroup\$ – xzyfer Feb 13 '11 at 1:03
  • \$\begingroup\$ @zxyfer: Haha -- didn't even notice that (I didn't copy/paste this exactly...) \$\endgroup\$ – Billy ONeal Feb 13 '11 at 1:49

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.