Skip to content

Instantly share code, notes, and snippets.

@scottrippey
Created August 24, 2012 17:43
Show Gist options
  • Save scottrippey/3453356 to your computer and use it in GitHub Desktop.
Save scottrippey/3453356 to your computer and use it in GitHub Desktop.
Code Review of FixedHeader.js
Uverse.widgets.FixedHeader = new Class({
Implements: [ Options ]
, el: null
, sectionsNavigationSelector: '.sections-navigation'
, _fixedZIndex: 15
, _sectionsNavigation: null
, _regularBodyPaddingTop: null
, _regularMastheadMarginTop: null
, _onScrollHandler: function() {
var bodyPaddingTop;
// If the scroll value is greater than the sectionsNavigation offset top
// we should fix it. The reason to use > instead of === is that browsers
// won't trigger this for every scrolled pixel.
if(window.scrollY > this._sectionsNavigation.offsetTop) {
// We change the style of `el`:
// - `width: 100%` because we want this to occupy all the screen wide.
// - `position: fixed` to fix it, duh.
// - `margin-top' is the weird one, we want to push the header top
// but we want to add padding to avoid a nasty "jump"
// caused by missing floating (fixing).
// - `z-index: 10` because we need to float over every element.
this.el.setStyles({
'width': '100%',
'position': 'fixed',
'margin-top': -2 * this._sectionsNavigation.offsetTop,
'z-index': this._fixedZIndex
});
// Set the body top padding to the offset of the navigation in order
// to avoid a "jump" caused by floating the masthead.
bodyPaddingTop = this._sectionsNavigation.offsetTop;
} else {
// If we are in a static safe zone, we can go ahead and return everything
// to the normal state.
this.el.setStyles({
'position': 'static',
'margin-top': this._regularMastheadMarginTop
});
bodyPaddingTop = this._regularBodyPaddingTop;
}
// @note: Should we move this inside the if statements?
document.body.setStyle('padding-top', bodyPaddingTop);
}
, initialize: function(el, options) {
this.el = $(el);
this.setOptions(options);
// Select the sections navigation…
this._sectionsNavigation = el.getElement(this.sectionsNavigationSelector),
// …and save the regular paddings and margins that will change on scroll.
this._regularBodyPaddingTop = document.body.getStyle('padding-top');
this._regularMastheadMarginTop = el.getStyle('margin-top');
document.addEvent('scroll', this._onScrollHandler.bind(this));
}
});
@scottrippey
Copy link
Author

Lines 22, 34: this.el.setStyles(...

We should not have so many styles in our JS.
Instead, we should use this.el.addClass("detached") and removeClass.
In the CSS, .detached should define z-index etc...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment