2
\$\begingroup\$

Write a simple monitoring navigation function.

  • I feel logic judgment still needs to be optimized.
  • I don't have a clear idea right now about code logic such as reducing if statements
  • How can I optimize the performance of this code?

<!-- =panel: start -->
<div class="panel" role="region"></div>
<!-- =panel: end -->

<script>
    var em = document.querySelectorAll(".panel");
    var viewHeight = window.innerHeight;
    var clientHeight = document.body.clientHeight;
    var timeOut = null;

    em.forEach(function (em, index) {
        watchNav(em);
    });

    function watchNav(em) {
        var emHeight = em.offsetHeight;
        var emTop = em.offsetTop;
        clearTimeout(timeOut);

        window.addEventListener("scroll", function (e) {
            var scrollY = window.scrollY;

            timeOut = setTimeout(function () {
                // Logical judgment: how to optimize, want better implementation.
                // Such as reducing the if statement
                // ++++++++++++++++
                if (scrollY > emTop || scrollY + viewHeight / 2 > emTop || scrollY + viewHeight > emTop + emHeight) {
                    console.log("...activing...");
                    em.classList.add("view-focus");
                }
                else {
                    console.log("none");
                    em.classList.remove("view-focus");
                }

                if (scrollY > emTop + emHeight / 2) {
                    console.log("none");
                    em.classList.remove("view-focus");
                }
                // ++++++++++++++++
            }, 40);
        }, false);
    }
</script>
\$\endgroup\$
  • 1
    \$\begingroup\$ .querySelectorAll() has a .forEach() method. Array.prototype.slice() is not necessary. See also Intersection Observer API \$\endgroup\$ – guest271314 Jan 9 at 8:34
  • 1
    \$\begingroup\$ Welcome, to Code Review, my kin. \$\endgroup\$ – FreezePhoenix Jan 9 at 15:59
0
\$\begingroup\$

A few comments:

  • Array.forEach takes a callback, this can be done directly as so: ems.forEach(watchNav)
  • Also, you have scope conflicts: the global variable em, and the function parameter em
  • A third comment: You should n ot edit your question based on feedback in comments.
  • Your if statement is fine.

<script>
    var ems = document.querySelectorAll(".panel"),
        viewHeight = window.innerHeight,
        clientHeight = document.body.clientHeight,
        timeOut = null;

    ems.forEach(watchNav);

    function watchNav(em) {
        var emHeight = em.offsetHeight;
        var emTop = em.offsetTop;
        clearTimeout(timeOut);

        window.addEventListener("scroll", function (e) {
            var scrollY = window.scrollY;

            timeOut = setTimeout(function () {
                // Logical judgment: how to optimize, want better implementation.
                // Such as reducing the if statement
                // ++++++++++++++++
                if (scrollY > emTop || scrollY + viewHeight / 2 > emTop || scrollY + viewHeight > emTop + emHeight) {
                    console.log("...activing...");
                    em.classList.add("view-focus");
                } else {
                    console.log("none");
                    em.classList.remove("view-focus");
                }

                if (scrollY > emTop + emHeight / 2) {
                    console.log("none");
                    em.classList.remove("view-focus");
                }
                // ++++++++++++++++
            }, 40);
        }, false);
    }
</script>
```
\$\endgroup\$
  • \$\begingroup\$ Thank you for your reply. I want to make the code more optimized. \$\endgroup\$ – Phoenix Jan 10 at 3:44
  • 1
    \$\begingroup\$ I would argue that a better way to thank someone is to up-vote the answer. \$\endgroup\$ – dfhwze Oct 11 at 4:29

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.