Moved all id, classname and jquery selector strings to "constants" objects

[d13]

This helps with future maintenance of the library as changing a classname or selector will only require a change in one place instead of many.

[alvarotrigo]

I can see it useful mainly in terms of compression, but the reason of making it easier to change doesn't convince me. It is almost as easy as using a "find an replace" and I don't see this variables as something that will be changed with frequency.

In any case, I would use real constants instead of objects, this way you could save more then 2KB (10%) in the minification and the syntax will be shorter and more easy to read.
I'm not either sure either of how to deal with the classNames variables. Having them by duplicate doesn't seem very reasonable.
A function could be used for it, but it would make the code more difficult to read and to be written.

[eamodio]

I think extracting these as constants is much more than just for compression (though that alone is a very valid argument imo). Not having "magic" strings all over the code helps with maintainability, refactoring, lessens the risk of typos/regressions, and could help contributers (and with taking contributions) as all the DOM selectors are now easily glance-able.

Though do agree that using raw vars (consts) would help the minification much more.

[alvarotrigo]

@eamodio yeah. I agree with you. Although I believe some of those strings shouldn't be constants, such as:

• body
• #fp-nav li
• #fp-nav li a
• .fp-tableCell, .fp-slidesContainer
• #fp-nav, .fp-slidesNav, .fp-controlArrow
• .fp-controlArrow.fp-next
• .fp-controlArrow.fp-prev

Having combinations of selectors which are only used in specific cases, such as the controls one ('#fp-nav, .fp-slidesNav, .fp-controlArrow) doesn't seem to be very useful for me when they can done by combining those constants.
I see it useful though for other cases such as activeSlide or activeSection which are widely used.

Then, not quite sure about the other combinations such as sectionNavLinks: '#fp-nav a',...

[eamodio]

@alvarotrigo Definitely agree with combining the constants rather than having combined constants, unless the combination really defines something that should be uniquely addressable (and used repeatedly).

[d13]

@alvarotrigo @eamodio - agree with individual constant variables and also separating combo selectors where used sparingly.

For selectors like 'body' and 'html, body', I think those should just be declared once and as jQuery objects since those shouldn't need to be re-looked up in the DOM.

[alvarotrigo]

For selectors like 'body' and 'html, body', I think those should just be declared once and as jQuery objects since those shouldn't need to be re-looked up in the DOM.

Ok. Fine then.

Should we just name the constants with upper cases without any prefix suffix to make them shorter and easier to read while coding?

For example: $(ACTIVE_SLIDE)

What about the class names? Should we just use a function for it?

function getClass(selector){
    return selector.substr(1)
}

$(SECTION).hasClass(getClass(ACTIVE_SLIDE));

What do you think about it?
It wouldn't look as clear in the code, but can't think about anything else.

[eamodio]

Using CAPS for the constants sounds good. But I would argue against using a function for conversion from selector to class or vice versa. My main argument would be around performance and the amount of strings that would get allocated and destroyed, which would put more pressure on the garbage collector. Causing more collections could cause more pauses in the experience.

I would suggest to just use un-prefixed CAPS variables for classes and prefixed (or post-fixed) CAPS variables for selectors that are the combination of the class variables.

var SLIDE = 'fp-slide';
var SEL_SLIDE = '.' + SLIDE;

or

var SLIDE = 'fp-slide';
var SLIDE_SEL = '.' + SLIDE;

I believe that would play well for readability, minification, and performance.

Thoughts?

[alvarotrigo]

Yeah, sounds reasonable.
I would prefer to use a single word for selectors, as they are used more in the code, but I think that adding the SEL prefix or postfix will be better in terms of readability. So I agree with you.

@d13 if you can update to the latest versoin 2.6.0 and make another pull request based on this comments I'll merge it.
And It would be great if you could create another PR for the pure javascript version of fullpage.js! :)

[d13]

Will do!

[d13]

@alvarotrigo this should be good to go

[alvarotrigo]

Missing replacement of $htmlBody in line 870.

[d13]

The line 870 fix was done on commit 9702c05.

SLIDE_PREV and SLIDE_WRAPPER_SEL references fixed.

[alvarotrigo]

Thanks for it!

[alvarotrigo]

I though it would compress the strings, but it seems it doesn't :D
At least, not Closure compiler. (YUI seems to do it)

Check out this example: http://jsfiddle.net/4d1jd1rk/

YUI compressor:

var i="active";var h=".fp-section";d(h).addClass(i);....

Closure compiler:

a(".fp-section").addClass("active");a(".fp-section").addClass("active");....

[d13]

I use UglifyJS2 (https://github.com/mishoo/UglifyJS2) for compression and it seems to do it.

[alvarotrigo]

Interesting:

From the FAQs:

Closure Compiler assumes that you are using gzip compression. If you do not, you should. Configuring your server to gzip your code is one of the most effective and easiest optimizations that you can possibly do. The gzip algorithm works by trying to alias sequences of bytes in an optimal way. Aliasing strings manually almost always makes the compressed code size bigger, because it subverts gzip's own algorithm for aliasing. So Closure Compiler will (almost) always inline your strings when it can, because that will make your compressed code smaller.