Permalink
Browse files

More bugfixes to voting, simplify events, use $var for jQuery vars

  • Loading branch information...
1 parent b6fc88c commit 7dcc3d411d0807eb09ac9371ccfa3b18adcfa6f4 @mc10 committed Oct 27, 2013
Showing with 62 additions and 56 deletions.
  1. +62 −56 lib/reddit_enhancement_suite.user.js
@@ -5605,7 +5605,7 @@ modules['keyboardNav'] = {
case 'comments':
// get all links into an array...
this.keyboardLinks = document.body.querySelectorAll('#siteTable .entry, div.content > div.commentarea .entry');
- if (!(isNew)) {
+ if (!isNew) {
this.activeIndex = 0;
}
break;
@@ -5619,21 +5619,23 @@ modules['keyboardNav'] = {
}
// wire up keyboard links for mouse clicky selecty goodness...
if ((typeof this.keyboardLinks !== 'undefined') && (this.options.clickFocus.value)) {
- for (var i=0, len=this.keyboardLinks.length;i<len;i++) {
- this.keyboardLinks[i].setAttribute('keyIndex', i);
- // changed parentElement to parentNode for FF3.6 compatibility.
- this.keyboardLinks[i].parentNode.addEventListener('click', (function(e) {
- var thisIndex = parseInt(this.getAttribute('keyIndex'), 10);
- if (modules['keyboardNav'].activeIndex != thisIndex) {
- modules['keyboardNav'].keyUnfocus(modules['keyboardNav'].keyboardLinks[modules['keyboardNav'].activeIndex]);
- modules['keyboardNav'].activeIndex = thisIndex;
- modules['keyboardNav'].keyFocus(modules['keyboardNav'].keyboardLinks[modules['keyboardNav'].activeIndex]);
- }
- }).bind(this.keyboardLinks[i]), true);
+ for (var i = 0, len = this.keyboardLinks.length; i < len; i++) {
+ $(this.keyboardLinks[i]).parent().data('keyIndex', i)
+ .on('click', modules['keyboardNav'].updateKeyFocus);
}
this.keyFocus(this.keyboardLinks[this.activeIndex]);
}
},
+ updateKeyFocus: function(event) {
+ event.stopPropagation();
@creesch

creesch Nov 12, 2013

bad bad RES killing poor poor toolbox

@andytuba

andytuba Nov 12, 2013

and link flair dialog and ...other things which have delegated event handlers attached above .thing

is this stoppropagation necessary @mc10? should this function be even called on click events?

@mc10

mc10 Nov 12, 2013

Owner

Hmm... I was trying to prevent the click-to-focus-post from bubbling up to the top, as it would mean that only the top-most comment would be highlighted. Perhaps delegating it to .commentarea will work.. let me try to fix this.

@andytuba

andytuba Nov 12, 2013

@mc10 2d4145a probably handles this, we're gonna test drive it for a bit and see if that clears up a bunch of issues related to event delegation

@mc10

mc10 Nov 13, 2013

Owner

@andytuba That seemed to have done the trick... hmm, interesting.

+
+ var thisIndex = parseInt($(this).data('keyIndex'), 10);
+ if (modules['keyboardNav'].activeIndex !== thisIndex) {
+ modules['keyboardNav'].keyUnfocus(modules['keyboardNav'].keyboardLinks[modules['keyboardNav'].activeIndex]);
+ modules['keyboardNav'].activeIndex = thisIndex;
+ modules['keyboardNav'].keyFocus(modules['keyboardNav'].keyboardLinks[modules['keyboardNav'].activeIndex]);
+ }
+ },
recentKey: function() {
modules['keyboardNav'].recentKeyPress = true;
clearTimeout(modules['keyboardNav'].recentKey);
@@ -12350,94 +12352,98 @@ modules['showParent'] = {
}
},
handleVoteClick: function(evt) {
- var voteClasses = { '-1': 'dislikes', 0: 'unvoted', 1: 'likes' },
- id = $(this).parent().parent().data('fullname'),
+ var $this = $(this),
+ voteClasses = { '-1': 'dislikes', 0: 'unvoted', 1: 'likes' },
+ id = $this.parent().parent().data('fullname'),
direction = /(up|down)(?:mod)?/.exec(this.className);
if (!direction) return;
direction = direction[1];
- var targetButton = $('.content .thing.id-' + id + ' > .midcol').find('.arrow.' + direction + ', .arrow.' + direction + 'mod');
- if (targetButton.length !== 1) {
+ var $targetButton = $('.content .thing.id-' + id + ' > .midcol').find('.arrow.' + direction + ', .arrow.' + direction + 'mod');
+ if ($targetButton.length !== 1) {
console.error("When attempting to find %s arrow for comment %s %d elements were returned",
- direction, id, targetButton.length);
+ direction, id, $targetButton.length);
return;
}
-
- targetButton.click();
+
+ $targetButton.click();
var clickedDir = (direction === 'up' ? 1 : -1),
- mid = $(this).parent(),
+ $midcol = $this.parent(),
startDir;
$.each(voteClasses, function(key, value) {
- if (mid.hasClass(value)) {
- startDir = key;
+ if ($midcol.hasClass(value)) {
+ startDir = parseInt(key, 10);
return;
}
});
var newDir = clickedDir === startDir ? 0 : clickedDir;
- mid.parent().children('.' + voteClasses[startDir])
+ $midcol.parent().children('.' + voteClasses[startDir])
.removeClass(voteClasses[startDir])
.addClass(voteClasses[newDir]);
- mid.find('.up, .upmod')
+ $midcol.find('.up, .upmod')
.toggleClass('upmod', newDir === 1)
.toggleClass('up', newDir !== 1);
- mid.find('.down, .downmod')
+ $midcol.find('.down, .downmod')
.toggleClass('downmod', newDir === -1)
.toggleClass('down', newDir !== -1);
},
showCommentHover: function(def, base, context) {
- var direction = modules['showParent'].options.direction.value;
- var thing = $(base);
+ var direction = modules['showParent'].options.direction.value,
+ $thing = $(base);
// If the passed element is not a `.thing` move up to the nearest `.thing`
- if (!$(thing).is('.thing')) thing = thing.parents('.thing:first');
- var parents = $(thing).parents('.thing').clone();
+ if (!$thing.is('.thing')) $thing = $thing.parents('.thing:first');
+ var $parents = $thing.parents('.thing').clone();
if (direction === 'up') {
- parents = $(parents.get().reverse());
+ $parents = $($parents.get().reverse());
}
- parents.addClass('comment parentComment').removeClass('thing even odd');
- parents.children('.child').remove(); // replies and reply edit form
- parents.each(function(i) {
+ $parents.addClass('comment parentComment').removeClass('thing even odd');
+ $parents.children('.child').remove(); // replies and reply edit form
+ $parents.each(function() {
+ var $this = $(this);
+
+ // Kill the keyboardNav functionality
+ $this.off('click');
+
// A link to go to the actual comment
- var id = $(this).attr('data-fullname');
+ var id = $this.data('fullname');
if (typeof id !== 'undefined') {
id = id.slice(3);
- $(this).find('> .entry .tagline').append('<a class="bylink parentlink" href="#' + id + '">goto comment</a>');
+ $this.find('> .entry .tagline').append('<a class="bylink parentlink" href="#' + id + '">goto comment</a>');
}
});
- parents.find('.parent').remove();
- parents.find('.usertext-body').show(); // contents
- parents.find('.flat-list.buttons').remove(); // buttons
- parents.find('.usertext-edit').remove(); // edit form
- parents.find('.RESUserTag').remove(); // tags
- parents.find('.voteWeight').remove(); // tags
- parents.find('.entry').removeClass('RES-keyNav-activeElement');
- parents.find('.author.userTagged').removeClass('userTagged'); // tags again
- parents.find('.collapsed').remove(); // unused collapse view
- parents.find('.expand').remove(); // expand button
- parents.find('form').attr('id', ''); // IDs should be unique
- parents.find('.arrow').prop('onclick', null); // clear the vote handlers
+ $parents.find('.parent').remove();
+ $parents.find('.usertext-body').show(); // contents
+ $parents.find('.flat-list.buttons').remove(); // buttons
+ $parents.find('.usertext-edit').remove(); // edit form
+ $parents.find('.RESUserTag').remove(); // tags
+ $parents.find('.voteWeight').remove(); // tags
+ $parents.find('.entry').removeClass('RES-keyNav-activeElement');
+ $parents.find('.author.userTagged').removeClass('userTagged'); // tags again
+ $parents.find('.collapsed').remove(); // unused collapse view
+ $parents.find('.expand').remove(); // expand button
+ $parents.find('form').attr('id', ''); // IDs should be unique
+ $parents.find('.arrow').prop('onclick', null) // clear the vote handlers
+ .on('click', modules['showParent'].handleVoteClick); // bind handlers to vote buttons
/*
I am stripping out the image viewer stuff for now.
Making the image viewer work here requires some changes that are for another time.
*/
- parents.find('.madeVisible, .toggleImage').remove(); // image viewer
- parents.find('.keyNavAnnotation').remove();
-
- var container = $('<div class="parentCommentWrapper">');
- container.append(parents);
- $(parents).slice(0, -1).after('<div class="parentArrow">reply to</div>');
- def.resolve("Parents", container);
+ $parents.find('.madeVisible, .toggleImage').remove(); // image viewer
+ $parents.find('.keyNavAnnotation').remove();
- // Bind handlers to vote buttons
- $('#RESHoverContainer').find('.parentCommentWrapper .arrow').on('click', modules['showParent'].handleVoteClick);
+ var $container = $('<div class="parentCommentWrapper">');
+ $container.append($parents);
+ $parents.slice(0, -1).after('<div class="parentArrow">reply to</div>');
+ def.resolve("Parents", $container);
}
};

0 comments on commit 7dcc3d4

Please sign in to comment.