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

This site is temporarily in read-only mode and not accepting new answers.

Not the answer you're looking for? Browse other questions tagged .