Jump to content
GreenSock

Learning

Improve my code?

Warning: Please note

This thread was started before GSAP 3 was released. Some information, especially the syntax, may be out of date for GSAP 3. Please see the GSAP 3 migration guide and release notes for more information about how to update the code to GSAP 3's syntax. 

Recommended Posts

Hi guys,

 

I have a simple caption for photos that can be hidden by a click of a button.
As my site has several of them, I decided to create a function to generate this effect across the different photos.

Using an empty object to hold the various items in each photo and click events to manipulate them.

But I felt it is a little scrappy using the object and am not sure if this is the best way to handle it.

There is no bugs or problems per se. I'm just wondering if anyone could help me suggest a better way to handle this functionality, or could this code be improved/shorten or anything? Just trying to learn to code better.
Thanks in advance guys!

 

// empty object to hold photos and their relevant parts
const eic = {};

function createExpandingButtons( pre, id ) {

	// store the various parts of the photo caption
	eic[ pre + 'Button' ]  = id.find( '.button' );
	eic[ pre + 'Arrow' ]   = id.find( '.button span' );
	eic[ pre + 'Caption' ] = id.find( '.caption' );
	eic[ pre + 'State' ]   = 1;

	eic[ pre + 'Button' ].on( 'click', { pre: pre }, function() {
		if ( eic[ pre + 'State' ] === 0 ) {
			eic[ pre + 'State' ] = 1;
			eic[ pre + 'Caption' ].css( 'position', 'relative' );
			TweenLite.to( eic[ pre + 'Arrow' ], 0.5, { rotation: -90, ease: Power4.easeOut } );
		} else {
			eic[ pre + 'State' ] = 0;
			eic[ pre + 'Caption' ].css( 'position', 'absolute' );
			TweenLite.to( eic[ pre + 'Arrow' ], 0.5, { rotation: 90, ease: Power4.easeOut } );
		}
	});
}


// example of creating one such photo caption
createExpandingButtons( '$indexHero', $( '#index-hero' ) );

 

Link to comment
Share on other sites

It's a little tough to know how to structure things best without knowing more about all the functionality you need, but I'll take a stab at a slight rewrite...

// empty object to hold photos and their relevant parts
const eic = {};
function createExpandingButtons( pre, id ) {
    // store the various parts of the photo caption
    var obj = {
        button: id.find( '.button' ),
        arrow: id.find( '.button span' ),
        caption: id.find( '.caption' ),
        state: true
    };
    eic[ pre ] = obj;
    obj.button.on( 'click', { pre: pre }, function() {
        //toggle the state
        obj.button.state = !obj.button.state;
        if (obj.button.state) {
            obj.caption.css( 'position', 'absolute' );
            TweenLite.to( obj.arrow, 0.5, { rotation: 90, ease: Power4.easeOut } );
        } else {
            obj.caption.css( 'position', 'relative' );
            TweenLite.to( obj.arrow, 0.5, { rotation: -90, ease: Power4.easeOut } );
        }
    });
}

// example of creating one such photo caption
createExpandingButtons( '$indexHero', $( '#index-hero' ) );

 

It just seemed a bit cleaner to be able to reference things like eic[ pre ].button, plus we're using a local "obj" variable that'll be faster to lookup. 

 

There are definitely ways to use ternary statements to make your code shorter, but that can also make it a bit less readable so it's not necessarily "better". 

 

Happy tweening!

  • Like 2
Link to comment
Share on other sites

Hi Jack!

Thanks for the suggestions.
Yes, I do think that wrapping the sub items into an object to reference seem a lot cleaner.
Will definitely use that!
 

I notice you're using a toggle state too, that feels a lot better!
I've always been using an if/else statement and then changing the states inside them with a = 1 and = 0 and that does feel clunky.
Will steal this to use everywhere! =D

 

Thanks!!!

  • Like 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
  • Recently Browsing   0 members

    • No registered users viewing this page.
×