Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed affix-bottom positioning #13541

Merged
merged 1 commit into from
May 10, 2014
Merged

Fixed affix-bottom positioning #13541

merged 1 commit into from
May 10, 2014

Conversation

gpakosz
Copy link
Contributor

@gpakosz gpakosz commented May 8, 2014

Hello,

When playing with an affixed sidebar, I noticed the sidebar could overlap my footer depending on the speed at which I scroll the page.

I also noticed that when I open Chrome dev tools and I make the dev tools panel take most of the height of the page, then refreshing the page would style my sidebar with .affix-bottom straight and also make it overlap my footer.

This is caused by this.$element.offset({ top: position.top }) which positions the affixed element depending on its current position which only works if you transition smoothly from .affix to .affix-bottom.

I suggest this line gets replaced with this.$element.offset({ top: scrollHeight - this.$element.height() - offsetBottom }) which corresponds to the computation made to decide whether or not to switch to .affix-bottom state.

I'm a first time Bootstrap user so please forgive me if I overlooked something obvious.
What do you think?

Set top position to (scrollHeight - this.$element.height() - offsetBottom).
@cvrebert cvrebert added the js label May 8, 2014
@gpakosz
Copy link
Contributor Author

gpakosz commented May 10, 2014

Actually, I just examined the history and the bug has been introduced by f7c360d with comment "Simplified calculation of the top offset when in the bottom state". By trying to simplify things, this commit introduces the bug I'm facing: fail to correctly position the affixed element when a page is refreshed while already being scrolled to bottom.

cc @sultano @fat

@fat
Copy link
Member

fat commented May 10, 2014

ah ok, looks good to me

@fat
Copy link
Member

fat commented May 10, 2014

thanks

fat added a commit that referenced this pull request May 10, 2014
Fixed affix-bottom positioning
@fat fat merged commit 0e91b85 into twbs:master May 10, 2014
@gpakosz gpakosz deleted the patch-2 branch May 10, 2014 16:43
@mdo mdo added this to the v3.2.0 milestone May 10, 2014
@mdo mdo mentioned this pull request May 10, 2014
@cvrebert
Copy link
Collaborator

X-Ref: #12862

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.

4 participants