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

Div's dynamic height conflicting with fixed (unscrollable) position #23

Open
iteralci opened this issue May 15, 2012 · 7 comments
Open

Comments

@iteralci
Copy link

We use your plugin for a menu with dynamic length (various accordions containing stuff like open elements, bookmarks, context actionss etc). It all works just fine, unless the menu becomes too long. Then, because of the fixed position, some elements in the open accordions become unreachable.
This is very similar to issue #22 where you introduced the option minWidth, so I made a little modification to checkScroll() (actually just the if-condition at l. 203):

 if (base.options.heightOffset && ($(window).height() < (base.options.heightOffset + target.height()))) {
     if (!isUnfixed() || !wasReset) {
          postPosition();
          target.trigger('preUnfixed');
          setUnfixed();
          target.trigger("unfixed");
        }
  }

so that the target's height is taken into account. heightOffset is a variable I introduced for better behavior (since the menu is below a header and just taking the target's height wouldn't be sufficient).

I thought you might want to integrate that.

@bigspotteddog
Copy link
Owner

Thanks for your contribution. I will look into integrating your changes as soon as I get a chance. Thanks, again.

@iteralci
Copy link
Author

I just noticed a little problem: if the menu is small enough upon page load, the position is set to static. If, without scrolling (which would set it to fixed), I open an accordion, isUnfixed() returns true and the spacer isn't resetted properly (this problem showed itself only in IE (8 and 9), where the remaning height attribute caused problems).

Can ScrollToFixed handle the div properly if the initial setting of position is fixed?

@bigspotteddog
Copy link
Owner

That's a good question. I have tested it with absolute, but not fixed from the start. So, the element is fixed, then at some point in the window scroll it is supposed to move up the page? Probably to reveal the longer menu items? Do I have the requirement correctly stated?

@bigspotteddog
Copy link
Owner

I just tested with an element that was originally fixed. I used a banner at the top of the page with position: fixed. The spacer is not added because the element is already fixed, as you have indicated. This is the initial condition, which is the same as the reset condition. So, I would say that, as of right now, the ScrollToFixed plugin does not work with the initial setting of position: fixed.

The trick is going to be distinguishing between an element that was originally fixed versus the same element that went fixed because of the plugin. The plugin uses the .css('position') !== 'fixed' value to determine when to reset the spacer. The position will return 'fixed' no matter where the scrollbar is and that is the problem.

I would need to synchronize a state variable and hope I can keep its value accurate to fix this.

@bigspotteddog
Copy link
Owner

I just tried a few things. None of them worked. I will need to give this more thought.

@iteralci
Copy link
Author

Sorry for the late reply.
The needed behavior is that the plugin deactivates the fixed position (like it does for horizontal scrolling) when the div gets longer than the window itself, as otherwise the elements below cannot be reached. This (I thought) I reached with the branch in my first post.
But if the menu gets longer than the window before the plugin sets the position to fixed (i.e. by opening an accordion just after the page load), the deactivation initiated by the new height doesn't reset the spacer properly.

This means, either the plugin can handle a fixed div from the beginning, so that isUnfixed() doesn't return true and the spacer is resetted properly, or the whole "deactivation routine" via postPosition(), triggers and setUnfixed() is made callable when the div is static without breaking anything.

@bigspotteddog
Copy link
Owner

OK, I think I understand. One of the things I played with was swapping lines 321 and 324. Line 321 is grabbing the position setting from the element, then line 324 is adding the spacer in place of the element, only if the element is not fixed, so no spacer in the cases where the element is fixed from the start. There is a variable that is tracking the target element's position, independently, already named "position." I was hoping that swapping those two lines would get the spacer to be added; however, the logic in the isUnfixed function needs to be adjusted to handle an undefined value.

That's about where I am at this point. I don't get a lot of time with the plugin lately because of other projects. I will keep at it as I have time available.

Thanks for all of your help with this.

bigspotteddog added a commit that referenced this issue Jan 15, 2013
…to determine whether or not to turn the plugin off; instead, of checking the user-agent for just iOS5. Added events for preUnfixed when the target hit its limit. Also, checked for the limit more accurately by using an originalPosition variable to record what the element's position was originally. Also, namespaced all events.
bigspotteddog added a commit that referenced this issue Jan 15, 2013
…to determine whether or not to turn the plugin off; instead, of checking the user-agent for just iOS5. Added events for preUnfixed when the target hit its limit. Also, checked for the limit more accurately by using an originalPosition variable to record what the element's position was originally. Also, namespaced all events.
bigspotteddog added a commit that referenced this issue Jan 15, 2013
…to determine whether or not to turn the plugin off; instead, of checking the user-agent for just iOS5. Added events for preUnfixed when the target hit its limit. Also, checked for the limit more accurately by using an originalPosition variable to record what the element's position was originally. Also, namespaced all events.
bigspotteddog added a commit that referenced this issue Jan 15, 2013
…to determine whether or not to turn the plugin off; instead, of checking the user-agent for just iOS5. Added events for preUnfixed when the target hit its limit. Also, checked for the limit more accurately by using an originalPosition variable to record what the element's position was originally. Also, namespaced all events.
bigspotteddog added a commit that referenced this issue Jan 15, 2013
…to determine whether or not to turn the plugin off; instead, of checking the user-agent for just iOS5. Added events for preUnfixed when the target hit its limit. Also, checked for the limit more accurately by using an originalPosition variable to record what the element's position was originally. Also, namespaced all events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants