Skip to content

Update js/bootstrap-affix.js#5017

Closed
kzxtreme wants to merge 1 commit intotwbs:masterfrom
kzxtreme:patch-1
Closed

Update js/bootstrap-affix.js#5017
kzxtreme wants to merge 1 commit intotwbs:masterfrom
kzxtreme:patch-1

Conversation

@kzxtreme
Copy link
Copy Markdown

@kzxtreme kzxtreme commented Sep 6, 2012

Patch for:
#4647

@andrewdeandrade
Copy link
Copy Markdown

Hey @kzxtreme,

Thanks for opening this pull-request! Unfortunately, it looks like it fails to pass the criteria neccessary for submitting to bootstrap. The following things are currently failing:

  • should always be made against -wip branches
  • should always include a unit test if changing js files

For a full list of issue filing guidelines, please refer to the bootstrap issue filing guidelines.

thanks!

@HariantoAtWork
Copy link
Copy Markdown

Guys I want to help, but I'm fairy new in git. So I don't know what the methods are.

I've download the code from https://github.com/twitter/bootstrap/zipball/master
And I've been working on the fix for js/bootstrap-affix.js

I did post something in the other thread 4647#issuecomment-9652919 but it the thread was closed. So I come to here.

Replace this

affix = this.unpin != null && (scrollTop + this.unpin <= position.top) ?
      false    : offsetBottom != null && (position.top + this.$element.height() >= scrollHeight - offsetBottom) ?
      'bottom' : offsetTop != null && scrollTop <= offsetTop ?
      'top'    : false

with these lines

var before_y_bool = offsetTop != null && scrollTop < offsetTop
    , inside_y_bool = scrollTop >= offsetTop && scrollTop <= scrollHeight-offsetBottom-this.$element.height()-(this.unpin != null ? this.unpin : 0)
    , before_offset_bottom_bool = offsetBottom != null && (position.top + this.$element.height() <= scrollHeight - offsetBottom)

affix = before_y_bool ? 'top' : (inside_y_bool && before_offset_bottom_bool) ? false : 'bottom'

The flickering cause by position.top. To make use of position.top, I make sure that within range of inside_y_bool.

Now I can finally sleep, so tired. Have been working this fix 4 long days.

@mdo
Copy link
Copy Markdown
Member

mdo commented Nov 29, 2012

Closing out for now—this issue should be resolved already.

@mdo mdo closed this Nov 29, 2012
@boekkooi
Copy link
Copy Markdown

@mdo I'm currently running in to this problem and the fix given by @HariantoAtWork works like a charm.

@albyrock87
Copy link
Copy Markdown

@boekkooi This makes two of us..
I'm facing the same issue and the fix given by @HariantoAtWork works fantastic!

@alejandr0bovino
Copy link
Copy Markdown

be 3, thanks!

@nostalgiaz
Copy link
Copy Markdown

The fix given by @HariantoAtWork works like a charm! Thanks!

@cvrebert
Copy link
Copy Markdown
Collaborator

If this is still a problem, I'd suggest opening a new issue. Just be sure to check whether v3 already fixes it or not.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants