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\$
1
  • 1
    \$\begingroup\$ livequery is the devil and needs to die in fire \$\endgroup\$
    – Raynos
    Jan 10, 2012 at 21:34

2 Answers 2

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\$
1
  • \$\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, 2012 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\$
3
  • 2
    \$\begingroup\$ LOL you just had to make it complicated :-P \$\endgroup\$
    – Naftali
    Jan 10, 2012 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, 2012 at 21:51
  • \$\begingroup\$ OMG :)))) you really got me good here :))) and yes, you are right sir. PS: OMG!!! \$\endgroup\$
    – Uffo
    Jan 10, 2012 at 22:00

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.