I have a text scoller that works well with Mozilla and Chrome and will play once on IE, but when the image is clicked, it freezes instead of replaying. Adding
}
else {
return
}
creates a syntax error.
Where I work, sites like jsfiddle are blocked, so I can’t see any of your code.
What about error messages in the console or Dev Tools (F12)? Anything there?
Hi WolfShade,
I don’t see any errors but I’m relatively new to all of this (just started learning CSS some months ago) and am struggling to process it all while trying to build a new web site. I’ve linked to a test page here which has the basic html and produces the issue. The JS for this is more complex than what I’ve been able to get by with in the past and I’m easily lost, but trying.
Many thanks!
The error occurs in IE8 because now - then results in NaN, brought on due to then being an invalid date string. Why is it invalid?
The problem occurs where 250 is added to a new date. IE8 turns the date in to a string and then adds “250” to the end of that string representation, which results in an invalid date.
A way that works is to use setMilliseconds and getMilliseconds, with something like this:
new Date().setMilliseconds(new Date().getMilliseconds() + 250);
Or, you can make it easier to understand by using the following function:
function addMilliseconds(date, ms) {
ms = Number(ms) || 0;
return date.setMilliseconds(date.getMilliseconds() + ms);
}
then = addMilliseconds(new Date(), 250);
Hi Paul,
Do I add new Date().setMilliseconds(new Date().getMilliseconds() + 250); in place of then = new Date() + 250;? I tried that and it does work, but on clicking the image the text either jumps to the end or starts from the beginning intermittently.
I also added
function addMilliseconds(date, ms) {
ms = Number(ms) || 0;
return date.setMilliseconds(new Date().getMilliseconds() + ms);
}
then = addMilliseconds(new Date(), 250);
in place of then = new Date() + 250; and it functions well, except that in IE8 the scroll slows down once the text reaches near the top of the window. That was pre-existing however.
Many Thanks!
Whoops, the function shouldn’t have new date() inside there, it should just be a reference to the date being passed in to the function.
return date.setMilliseconds(date.getMilliseconds() + ms);
but that won’t affect the running of the script as you currently have it.
After making suitable changes to the jsfiddle code, things seem to run a lot better when using IE’s developer pane to run things as IE8. I don’t seem to experience that slowing down when it reaches near the top though - I’d like to see a page that exhibits that problem.
Hi Paul,
I very much appreciate the clarification.
I’ve update the test page although I’m certain I’ve not added the script correctly which is likely why I am seeing the slowdown of the scroll as the text nears the top.
Many Thanks!
When the page content reaches the top of the window, that’s when things slow down on IE. It’s not related to the version of IE either.
I’ll see what I can find out about this.
Ahh - it is a browser version issue afterall. IE uses only integer CSS values in IE8, and after that allows decimal values. As a result - you cannot rely on reading the correct CSS value.
We’ll update the top variable so that it tries to get from itself, and only going to the CSS value if top is falsy.
top = top || parseFloat( content.style.top );
Now we can set the top value, and use that to set the CSS style.
top = top - step;
content.style.top = top + 'px';
which results in a much improved situation on IE8.
Hi Paul,
Yes, it does scroll very smoothly now. I have done something wrong however as the text jumps to the top when the image is clicked to restart.
Very appreciative for the learning experience. Thanks!
At the other places that style.top is updated, you also need to first set the top variable, and then assign it to style.top
For example:
Before the update:
content.style.top = -(contentHeight - contentOffset) + 'px';
After the update:
top = -(contentHeight - contentOffset);
content.style.top = top + 'px';
Then check for any other situations where you update style.top. Whenever you find one, save the top value, and then use it to set the style as in the example above.
I am embarrassed having missed that! Early morning at the time and only one cup of coffee. Well, that and still green with this stuff. I think I have it right now. It’s working anyway, but would you please tell me if there are any mistakes or redundancies in what I’ve done?
Many, many thanks!
It’s clear from the code that there’s quite a lot of duplication in regard to setting the top style, so let’s pull that out to a separate function.
function setTop(el, styleTop) {
top = styleTop;
el.style.top = top + 'px';
}
...
setTop(content, top - step);
Also, the addEventListener and attachEvent functions are nearly identical, which can be made identical, and then refactored out as a separate function called tickUpdate
function tickUpdate() {
setTop(content, contentOffset);
then = addMilliseconds(new Date(), tickDelay);
setTimeout(tick, tickDelay);
}
and now the code to add that tickUpdate is looking near-identical to the attachEvent compatibility code, so we can place that in to a well defined function too:
function addEventTo(el, type, func) {
if (el.addEventListener) {
el.addEventListener(type, func, false);
} else if (el.attachEvent) {
el.attachEvent('on' + type, func);
}
}
The main benefit of extracting code out to functions such as the above, is that it makes the remaining code easier to understand:
tick();
addEventTo(document.querySelector('[data-scroller-restart]'), 'click', tickUpdate);
After a few other tidy-ups we can also improve on things further thanks to JSLint. After cleaning up issues that are found, we end up with the following script:
var scroller = function (scroller) {
if (!(scroller instanceof Element)) {
throw new TypeError('[scroller] must be a DOM element');
}
scroller.innerHTML = '<div data-scroller-contents>' + scroller.innerHTML + '</div>';
var content = scroller.firstChild,
contentHeight = content.offsetHeight,
contentOffset = 130,
tickDelay = 250,
now,
then = new Date(),
top;
function setTop(el, styleTop) {
top = styleTop;
el.style.top = top + 'px';
}
function addNewTick(func) {
if (window.requestAnimationFrame) {
window.requestAnimationFrame(func);
} else {
setTimeout(func, 100);
}
}
function tick() {
now = new Date();
var step = (now - then) / 60;
top = top || parseFloat(content.style.top);
if (top > contentOffset - contentHeight) {
setTop(content, top - step);
then = now;
addNewTick(tick);
} else {
setTop(content, contentOffset - contentHeight);
}
}
function addMilliseconds(date, ms) {
ms = Number(ms) || 0;
return date.setMilliseconds(date.getMilliseconds() + ms);
}
function tickUpdate() {
setTop(content, contentOffset);
then = addMilliseconds(new Date(), 250);
setTimeout(tick, tickDelay);
}
function addEventTo(el, type, func) {
if (el.addEventListener) {
el.addEventListener(type, func, false);
} else if (el.attachEvent) {
el.attachEvent('on' + type, func);
}
}
/* important styles (you can set anything else in your css, or even leave it empty) */
scroller.style.position = 'relative';
scroller.style.overflow = 'hidden';
content.style.position = 'relative';
setTop(content, contentOffset);
/* start scrolling */
tick();
addEventTo(document.querySelector('[data-scroller-restart]'), 'click', tickUpdate);
};
scroller(document.querySelector('[data-scroller]'));
Further improvements can be made from here too, but things are a lot better now than when compared with before.
Hi Paul,
I really appreciate cleaning the script up. I tried a couple of the online sites to tidy it up but it stopped running thereafter. Seeing what you did and pointing me to JSLint helps me understand, and try it again for myself.
I’ve learned a lot from this and am grateful to you for it.
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.