1
\$\begingroup\$

I have created a piece of code for a live pagination, but I don't like the code. It works it does the job, but I guess I could done it better.

$('.show-content').livequery(function() {
    $(this).find('.page-me:not(:first)').hide();
    var totalPage = $(this).find('.page-me').length;

    for (var i = 0; i <= totalPage - 1; i++) {
        $('.modal-pagination').append('<a href="#" id="' + i + '">Page ' + (i + 1) + '</a>')
    }

    $('.modal-pagination a').live('click', function() {
        var thePag = $(this).attr('id');

        $('.show-content').find('.page-me').hide();
        $('.show-content').find('.page-me:nth(' + thePag + ')').show();

        return false;
    });
});
\$\endgroup\$

migrated from stackoverflow.com Jan 10 '12 at 21:24

This question came from our site for professional and enthusiast programmers.

  • 1
    \$\begingroup\$ livequery is the devil and needs to die in fire \$\endgroup\$ – Raynos Jan 10 '12 at 21:34
3
\$\begingroup\$

The code looks fine to me. The only things I would personally change is the use of the live() method (which has been deprecated, and was slow anyway) todelegate() and the use of the :nth() selector, which again I believe is not very fast - although I've not specifically tested it. In place of that I'd use eq().

$('.modal-pagination').delegate('a', 'click', function(e) {
    e.preventDefault();
    var thePag = $(this).attr('id');
    $('.show-content').find('.page-me').hide().eq(thePag -1).show();
});
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot! Please remove thePag -1 I have taken care of that in the loop, I will accept it asap. \$\endgroup\$ – Uffo Jan 10 '12 at 21:41
2
\$\begingroup\$
var toArray = [].slice.call.apply([].slice]);
var contentShow = toArray(document.getElementsByClassName('contentShow'));
contentShow.forEach(function doStuff(element) {
    var pageMe = toArray(element.getElementsByClassName('page-me')),
        modalPagination = toArray(document.getElementsByClassName));

    pageMe.unshift();
    pageMe.forEach(function hideElement(element, index) {
        element.classList.add('hidden');
        modalPagination.forEach(function addLink(modal) {
            var link = document.createElement("a");
            link.id = index;
            link.textContent = 'Page ' + (index + 1);
            modal.appenChild(link);
        });
    });

    modalPagination.forEach(function delegateClick(element) {
        element.addEventListener('click', function handleClick() {
            if (this.tagName !== 'a') return;

            var thePage = this.id;    

            var showContent = toArray(document.getElementsByClassName('show-content'));
            showContent.forEach(function doStuffWithPageMe(element) {
                var pageMe = toArray(element.getElementsByClassName('page-me'));
                for (var i = 0, len = pageMe.length; i < len; i++) {
                    var page = pageMe[i];
                    if (i % thePage === 0) {
                        pageMe.classList.remove('hidden');    
                    } else {
                        pageMe.classList.add('hidden');
                    }
                }
            });
        } 
    });
});

The code is ugly because your logic is ugly. You need to fix your logic and also probably fix your HTML.

\$\endgroup\$
  • 2
    \$\begingroup\$ LOL you just had to make it complicated :-P \$\endgroup\$ – Neal Jan 10 '12 at 21:47
  • 2
    \$\begingroup\$ @Neal No, I made the complexity jQuery hides be shown in plain sight. If you don't see what the code really does you would still be disillusioned into thinking the code isn't bad. \$\endgroup\$ – Raynos Jan 10 '12 at 21:51
  • \$\begingroup\$ OMG :)))) you really got me good here :))) and yes, you are right sir. PS: OMG!!! \$\endgroup\$ – Uffo Jan 10 '12 at 22:00

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.