patternjavascriptMinor
Show and hide posts using setTimeout
Viewed 0 times
showsettimeoutusingandpostshide
Problem
I have made an effect that has the
h1 move and then reveals p using .addClass, .removeClass and CSS3 transition. It works perfectly but I think I am repeating too much of the code and I think there may be a way to write it better. I need postShow and postHide to be different because I need setTimeout to be in postShow for timing. Please review the code below and give me any pointers.jQuery(document).ready(function($){
var post = $('div.post');
var postShow = function(item) {
$(item).closest('a').find('img.postimg').addClass('imggrow');
$(item).closest('a').find('h1').addClass('bannerhover');
setTimeout(function(){
$(item).closest('a').find('p.excerpt').addClass('phover');
},500);
};
var postHide = function(item) {
$(item).closest('a').find('img.postimg').removeClass('imggrow');
$(item).closest('a').find('h1').removeClass('bannerhover');
$(item).closest('a').find('p.excerpt').removeClass('phover');
};
//-----------------------------------------------------------
if ($(window).width() > 1000) {
post.hover(function() {
$(this).find('a').find('p.excerpt').show(0, function() {
postShow(this);
});
},function() {
$(this).find('a').find('p.excerpt').hide(0, function() {
postHide(this);
});
});
}
//--------------------------------
});Solution
Firstly, you should add a class to the
That way, you don't have to add/remove classes on all the child elements separately. You can just add/remove 1 class, and set up your CSS to style the child elements accordingly.
You also decouple your JS from the HTML to greater extent; now you can control things in just the CSS instead of having the HTML, the CSS, and the JS all concern themselves with the exact structure of your page.
Given your current code, however, I would suggest something like this:
a element instead, since that's what all the other elements are contained in.That way, you don't have to add/remove classes on all the child elements separately. You can just add/remove 1 class, and set up your CSS to style the child elements accordingly.
You also decouple your JS from the HTML to greater extent; now you can control things in just the CSS instead of having the HTML, the CSS, and the JS all concern themselves with the exact structure of your page.
Given your current code, however, I would suggest something like this:
jQuery(function($) { // shortcut for $(document).ready(...)
function showPost() {
var a = $(this).closest("a"); // get this once, use it thrice
a.find('img.postimg').addClass('imggrow');
a.find('h1').addClass('bannerhover');
a.find('p.excerpt').delay(500).addClass('phover'); // note the delay() call
}
function hidePost() {
var a = $(this).closest("a"); // same as above
a.find('img.postimg').removeClass('imggrow');
a.find('h1').removeClass('bannerhover');
a.find('p.excerpt').removeClass('phover');
}
if ($(window).width() > 1000) {
// No need to use hide/show with a timeout of zero, as far as I can tell.
// Just pass the two functions directly
$('div.post').hover(showPost, hidePost);
}
});Code Snippets
jQuery(function($) { // shortcut for $(document).ready(...)
function showPost() {
var a = $(this).closest("a"); // get this once, use it thrice
a.find('img.postimg').addClass('imggrow');
a.find('h1').addClass('bannerhover');
a.find('p.excerpt').delay(500).addClass('phover'); // note the delay() call
}
function hidePost() {
var a = $(this).closest("a"); // same as above
a.find('img.postimg').removeClass('imggrow');
a.find('h1').removeClass('bannerhover');
a.find('p.excerpt').removeClass('phover');
}
if ($(window).width() > 1000) {
// No need to use hide/show with a timeout of zero, as far as I can tell.
// Just pass the two functions directly
$('div.post').hover(showPost, hidePost);
}
});Context
StackExchange Code Review Q#110953, answer score: 3
Revisions (0)
No revisions yet.