1
\$\begingroup\$

This is a revision of a previous question that I asked (i.e. Check browser compatibility for RequestAnimationFrame and Vanilla JavaScript .animate() API), with new aspects that I have learnt in Javascript. It is I think better than the previous implementation as it is using switch statement's which will only run if certain conditions are met, which means a performance gain I guess. Can anyone spot any issues with what I have so far? A link to the original Question is posted at the bottom of this one for reference.

        const LoadScreen = document.getElementById("Temp");
        const LoadRing = document.querySelector(".loader");

        setTimeout(() => {
            LoadScreen.style.display = "none";
            LoadRing.style.display = 'none';
        }, 875);

   let webAnimSupport = (window.Element.prototype.animate !== undefined);
   let rafSupport = (window.requestAnimationFrame !== undefined);
   let cssFallback = false;

   function animeFallBack(){
       switch(rafSupport ? true : false){
           case true:
                console.log('.raf(); Support = true');
                runRAFAnime();
                // .animate Not Supported Fallback to `request animation frame`.
                //   Add Callback or Promises in here which holds RAF Anime Code.
                //   Put the RAF Code in an External File to reduce the size of this one.
                //   When Learnt more move the Animations into there Own WebWorker...
           break;
           case false:
                console.log('.raf(); Support = false');
                let requestAnimationFrame = (
                    window.requestAnimationFrame ||
                    window.mozRequestAnimationFrame ||
                    window.webkitRequestAnimationFrame ||
                    window.msRequestAnimationFrame ||
                    function(callback) {
                        return window.setTimeout(callback, 1000 / 60)
                    }
                );
                // Fallback option or alert enable Js
                //  Wrap this in a Try Catch Block & fallBack to Css Anime.
           break;
           default:    // Default to Css Fallback.
            var AnimeStyle = document.createElement("link");    // Default Classes to be added back in.
                AnimeStyle.setAttribute("rel", "stylesheet");
                AnimeStyle.setAttribute("type", "text/css");
                AnimeStyle.setAttribute("href", "FallBack.css");
                document.getElementsByTagName("head")[7].appendChild(AnimeStyle);
                return false;
       }
   }

   switch(webAnimSupport ? true : false){
       case true:
            console.log('.animate(); Support = true');
            RunAnimeApi();
              // Run .animate() functions as normal via Callbacks or Promises.
       break;             // break; 4 WebAnimSupport = True;
       case false:
            console.log('.animate(); Support = false');
            animeFallBack();
                     // Move onto requestAnimationFrame();
       break;        // break; 4 WebAnimSupport = False;
       default:
          return false;
                     // Default to Css Fallback. ie ``Add Back in the Classes`` That governed the original Animation.
   }

Ideally I would like to include promises within this block but these are something I'm still struggling to wrap my head around. Can anybody spot any mistakes, performance gains or security issues with this current implementation?

Related Articles:
    paul irish polyfill
    .animate polyfill();
    requestAnimationFrame(); polyfill
    Game Loop Crashing my Browser
    requestAnimationFrame polyfill produce a timestamp aswell
    requestAnimationFrame polyfill error in jest tests
    Do we need to clear a timeout when using requestAnimationFrame polyfill
    requestAnimationFrame polyfill argument
Maybe Related:
    Check if two elements exist in an array with older browser support
    mozilla.org/en-US/docs/Web/API/Element/animate
    Waapi better api web animations
Kind of related:
    What is the best polyfill for requestanimationframe out there
    Npm request-animation-frame-polyfill
    Ember Cli requestAnimationFrame Polyfill

Test Browser Support in Older Broswers:
    browsershots.org
    browserling.com
    sitepoint.com/cross-browser-testing-tools

Will Update soon.

\$\endgroup\$
  • \$\begingroup\$ You put a bounty on your question, but the end of the question still states "Will Update soon". Please note that modifying the question after answers come in can be problematic. Doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast Mar 11 at 6:40
  • \$\begingroup\$ @Mast ok, thanks if i find a more optimal solution I will repost it as a new question & link to it here.. \$\endgroup\$ – Ryan Stone Mar 11 at 10:37
  • \$\begingroup\$ @Mast, oh actually. only ment that I'd update the links section if and when i find more resources on the subject.. \$\endgroup\$ – Ryan Stone Mar 11 at 10:38
4
+50
\$\begingroup\$

Like I mentioned in my answer to your post Check browser compatibility for RequestAnimationFrame and Vanilla JavaScript .animate() API, many features like arrow functions and the let and const keywords are used, which cause errors in some browsers the code aims to target - e.g. IE 10 and older. So instead of an arrow function in the callback to setTimeout() use a regular function expression or the name of a function declared with a function declaration, and instead of the const and let keywords, use var.


const LoadRing = document.querySelector(".loader");

Is there only one element with that class name? If so, it would likely be more appropriate to use an id selector instead of a class name to select it.


switch(rafSupport ? true : false){

rafSupport is a boolean (see snippet below for proof) and thus there is no need to use the ternary expression here

let rafSupport = (window.requestAnimationFrame !== undefined);
console.log('typeof rafSupport: ',typeof rafSupport )

The whole switch statement seems like overkill for a boolean condition - most developers would stick to if/else statements.


default:    // Default to Css Fallback.
            var AnimeStyle = document.createElement("link");    // Default Classes to be added back in.
                AnimeStyle.setAttribute("rel", "stylesheet");
                AnimeStyle.setAttribute("type", "text/css");
                AnimeStyle.setAttribute("href", "FallBack.css");
                document.getElementsByTagName("head")[7].appendChild(AnimeStyle);
                return false;

This section of the switch statement is unreachable because:

  • the previous cases are true and false, and
  • the previous cases both contain break at the end

And are there really 8+ elements with the tag name head??


default:
      return false;
                 // Default to Css Fallback. ie ``Add Back in the Classes`` That governed the original Animation.

This leads to an error:

'return' statement outside of function

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Yes the arrow functions maybe an issue. not sure how to fix this yet. will research or just remove them.. No, i didn't notice that head issue i ment to input the css after the 8th meta/link tag ie after charset, title etc. will look into fixing this aswell \$\endgroup\$ – Ryan Stone Mar 10 at 18:42
  • \$\begingroup\$ would if(!rafSupport){...code} be more optimal.. \$\endgroup\$ – Ryan Stone Mar 10 at 18:45
1
\$\begingroup\$

Sᴀᴍ Onᴇᴌᴀ has already pointed out several major issues in your code. Besides those, your code also includes unused variables (e.g. cssFallback) and hard-to-read back and forth control flow (since, for some reason, you've decided to stick the animeFallBack function between the definition of webAnimSupport and the switch statement that uses it and possibly calls the function). Your code also has several likely bugs, as pointed out by Sᴀᴍ Onᴇᴌᴀ, and it's not even a complete, testable program (which means that there might well be more bugs that we can't easily spot). In fact, it's doubtful whether your code even qualifies as "working as intended", as required by the Code Review SE topic guidelines.

Based on what you've written in your question, it also looks like you probably don't really understand how basic JavaScript syntax features like if and switch statements actually work. Meanwhile, you've listed a whole bunch of "related articles" in your question, all of which appear to be dealing with considerably more advanced topics than basic JavaScript syntax, and which are probably taking a working knowledge of such syntax for granted.

To be brutally honest, it looks like you've jumped into the deep end of the pool before you've learned how to swim, and that you're now basically just copy-pasting and mixing up random bits of code that you've found online together without actually understanding them, and hoping that they'll somehow magically work.

So my suggestion for you would be to start by finding a good and comprehensive JavaScript tutorial for beginners — I'd personally recommend a printed book, as they're more likely to come as a complete and self-contained package, although there are some fairly decent online tutorials around, too — and reading through it. Since you say you're interested in maintaining compatibility with older browsers, an old pre-ES6 tutorial would suit you fine, although a modern tutorial that covers both styles would be OK too.

You can find plenty of Javascript tutorials — both printed books and online — via Google. I'm not going to even try to provide any specific recommendations, although the first result I got (javascript.info) at least looks fairly decent at a glance (and seems to cover both pre-ES6 and modern syntax, although with an emphasis mostly on the latter).

Once you've picked up a better grasp of the basics, you should be able to come back and apply the suggestions from the references you've listed in your question to come up with a clean and working solution. FWIW, it'll probably end up looking a lot like the code in Adam Taylor's answer to your earlier question.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, Duly noted.... \$\endgroup\$ – Ryan Stone Mar 11 at 20:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.