Closed VIPStephan closed 9 years ago
How does clearing floats cause issues?
I have headings with a child that is dragged upwards with negative top margin and pushes the heading back down with equal positive bottom margin so that the heading text is below the fixed header, not behind it. Clearing floats on these headings (or parents) creates a space above these headings (after any floated content that comes before these) as large as that previously mentioned margin which is undesirable. So, I’d rather specify an actuall scrollTop value with JS, that makes a lot of things easier.
I hope what I wrote was comprehensible. Thanks for your quick reply.
Hard to visualize what you are describing without a link.
I’ve uploaded a sketchy version at http://vipstephan.de/p/temp.html I’ve added a inline style to the “Measuring Progress” headline to illustrate the problem. The other headings are not cleared and are nicely spaced out (and are flowing around floated content – you can see this if you remove the inline clear from that one heading with a developer console).
I'm sorry still not clear from your example what the problem is. So you don't want the heading to clear the floated content?
Sorry for the confusion. I do want the headings to clear the floats but this adds unnecessary space to them. I’ve updated my example above by giving the headings a green background color. You see that they are actually larger than they seem, to accomodate for the fixed header. The top of non-cleared headings is dragged upwards but as soon as I clear the headings (such as that one “Measuring Progress”) or any of their containers (such as the section they are part of) their top edge is moved below the floated content, adding too much space. Therefore I cannot use the CSS method of adding a scroll offset, I need to explicitly scroll the page to a certain value (i. e. the top offset of the heading plus the height of the fixed element), calculated by JavaScript.
I hope that makes it clearer.
You've implemented the CSS trick incorrectly. http://davist11.github.io/jQuery-One-Page-Nav/top.html
To account for the offset of the horizontal nav, add top negative margin and top padding.
So just add that to your sections (the same size as your floating heading), and everything works fine.
This is not the solution, though, because you have nested multiple divs and have the IDs on the section wrappers, which I cannot do. If I add a floated container to a section in your example and clear the section wrapper the same problem occurs. Your approach relies on a wrapper whose sole purpose is to serve as the anchor and which can never be cleared.
Can’t you just accept that there are cases where a dynamic calculation of the scroll position is more appropriate/feasible than the CSS method? What harm does it do to keep scrollOffset in the code (except a few bytes of added file size)?
You are more than welcome to use a previous version of the plugin. I have no plans to add that back in.
if anyone is wondering, you can easily add it back by changing the scrollTo method around line 202 to
scrollTo: function(target, callback) {
var offset = $(target).offset().top;
$('html, body').animate({
scrollTop: offset - this.config.scrollOffset, // this is the important line
}, this.config.scrollSpeed, this.config.easing, callback);
},
and add scrollOffset: 0
to the defaults around line 36
OnePageNav.prototype = {
defaults: {
navItems: 'a',
currentClass: 'current',
changeHash: false,
easing: 'swing',
filter: '',
scrollOffset: 0, // this is the important line
scrollSpeed: 750,
scrollThreshold: 0.5,
begin: false,
end: false,
scrollChange: false
},
btw @davist11 the fix (negative margin and padding) you're forcing onto devs complects styling and layout with plugin implementation (while also forcing hacky definitions in your users code). plugins should live completely separate from all other implementation detail; there's a reason why most other scrolling plugins provide an optional offset.
@burabure appreciate the opinion, but using a negative margin and padding has no impact on the style of the page. The scrollOffset
option does not work with changeHash
, which is why it was removed.
I have a case where the CSS offset method isn’t feasible. For example, it becomes problematic when floats are cleared. I really need to calculate the offset dynamically and scroll accordingly. scrollOffset would have been perfect for that but I noticed that it has been removed previously. It doesn’t hurt to have it, though; at least people have the choice to either use it or ignore it and use CSS instead.