0
\$\begingroup\$

This question was originally posted on Stack Overflow.

You can see the before and after there, but here's the after that I'd like to bring for review here.

The goal of this script is to polyfill any browsers I'm supporting that don't natively have position: sticky. Example: I'm thinking about detecting IE11 and running this script only there.

My questions then are:

  1. Does structuring this code as a class makes sense here?
  2. Is there anything obvious about my class that's wrong?
  3. How should I export this as an ES6 module?
  4. Any other advice?

Code structure is the one big thing I struggle with with JS, and so I'd greatly appreciate any feedback you have.

class makeSticky {
	constructor(el) {
		this.element = el;
		this.fixedClass = 'is-fixed';
		this.parent = this.element.parentNode;
		this.position = this.element.offsetTop;
		this.parentBottom = this.parent.clientHeight + this.parent.offsetTop;
	}

	init() {
		this.addEvents();
		this.onScroll();
	}

	addEvents() {
		window.addEventListener('scroll', this.onScroll.bind(this));
	}

	onScroll(event) {
		if (this.aboveScroll() && this.stillInsideParent()) {
			this.setFixed();
		} else {
			this.setStatic();
		}
	}

	aboveScroll() {
		return this.position < window.scrollY;
	}

	stillInsideParent() {
		return this.parentBottom > window.scrollY;
	}

	onScroll(event) {
		if (this.aboveScroll() && this.stillInsideParent()) {
			this.setFixed();
		} else {
			this.setStatic();
		}
	}
	setFixed() {
		this.element.classList.add(this.fixedClass);
	}
	setStatic() {
		this.element.classList.remove(this.fixedClass);
	}
}

const children = document.querySelectorAll('.child');
children.forEach(child => {
	const sitck = new makeSticky(child);
	sitck.init();
});.
  .wrapper {
    max-width: 800px;
    margin: 60px auto;
  }

  .is-flex {
    display: flex;
  }

  .sidebar {
    width: 300px;
    flex: none;
    background: #eee;
  }

.child {
  color: red;
}
  .is-fixed {
    position: fixed;
    top: 20px;
  }
<div id="app">

			<div class="wrapper is-flex parent">
				<div id="main" class="main">
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
				</div>

				<div class="sidebar">
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<h2 class="child first">Title one should be sticky</h2>
				</div>

			</div>

			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>



			<div class="wrapper is-flex parent">
				<div class="main">
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
					<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.</p>
				</div>

				<div class="sidebar">
					<h2 class="child two">Title two should be sticky</h2>
				</div>

			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
			<div class="wrapper">
				<h2>A full width thing might go down here before another wrapper starts</h2>
			</div>
		</div>

\$\endgroup\$
1
2
\$\begingroup\$

Review

First up Welcome to code review :)

A quick note that the posted code is broken, it throws an error when the snippet runs, and the object makeSticky has a duplicated property onScroll

As you are new I will say you don't have to accept the first answer that comes along, give it a bit of time as there is more than one school of thought in programing, and questions that have accepted answers don't attract as many new answers.

On to the review.

Your questions

Does structuring this code as a class makes sense here?

No. You keep no reference to it, thus there is no need for an interface. Only the onScroll function need access to the element and that can be done with closure.

How should I export this as an ES6 module?

If you plan to use it on more than one page then you can wrap it in a function and export that function. Depending on how you plan to use it you can either pass it a list of elements or let it find the elements. See examples below.

Is there anything obvious about my class that's wrong?

Any other advice?

Read on...

Style points

  • window is the default object (the global this) and is not needed. . You don't use it for window.document, so why use it window.scrollY and window.addEventListener If you wish to use it there are some edge case where there is a benefit but you should be consistent.
  • If you are not using an argument don't declare it. onScroll(event) { is just as good as onScroll() {
  • Functions containing one line of code called only from one place, adds 3 lines of code for no reason. A general rule of thumb for functions is that A function should reduce the overall code size not increase it.
  • makeSticky is not a good name, you generally don't start an object's name with a verb, maybe stickyElement or just sticky

Too much bloat

Don't add what is not needed. You define an object makeSticky and give it 13 exposed (public) properties. When you instantiate it you immediately drop the reference so that the only code that can access the object, is its self, making all the properties and supporting code redundant..

By the looks you only ever create this object in one place, so there really is no need for it to be named in the global scope. And as the object is not referenced elsewhere it can exist as just a closure over the onScroll listener.

Keep it simple

When writing you should be constantly aware of what is needed and by what.

  • Don't add code that will never be used,
  • Don't add code for some possible imagined future need, only what is needed (unless its specifically outlined in the design).
  • Try to reduce the number of times you need to use this Each this means an exposed property, more support code, and greater risk of the object state being mutated.

Examples

As only the onScroll event listener needs access to the elements all the following examples use closure to maintain the reference. There is no named object for what you called makeSticky

Short and simple

document.querySelectorAll('.child').forEach(child => {
  const fixed = 'is-fixed',  top = child.offsetTop;
  const bot = child.parentNode.clientHeight + child.parentNode.offsetTop;
  const onScroll = () => child.classList(top < scrollY && bot > scrollY ? "add" : "remove")(fixed);
  addEventListener('scroll', onScroll);
  onScroll();        
});

or you may prefer a more open style

document.querySelectorAll('.child').forEach(el => {
  const fixed = 'is-fixed';
  const top = el.offsetTop;
  const bot = el.parentNode.clientHeight + el.parentNode.offsetTop;
  addEventListener('scroll', onScroll);
  (function onScroll() {
    if (top < scrollY && bot > scrollY) { 
      el.classList.add(fixed) 
    } else {  
      el.classList.remove(fixed);
    }
  })();
});

As a module

/* in js file sticky.js */
export default function sticky(selection) {
  selection.forEach(el => {
    const fixed = 'is-fixed',  top = el.offsetTop;
    const bot = el.parentNode.clientHeight + el.parentNode.offsetTop;
    const onScroll = () => el.classList(top < scrollY && bot > scrollY ? "add": "remove")(fixed);
    addEventListener('scroll', onScroll);
    onScroll();        
  });
}

/* In another script */
import sticky from "sticky";
sticky(document.querySelectorAll('.child'));
\$\endgroup\$
5
  • \$\begingroup\$ Wow! Holy cow, what an introduction to this site that I'd never heard about! Thank you for a tremendous answer! Do you happen to know of any books/resources that address some of the issues that are plain to see in my code? A lot of my JS troubles stem from just knowing some code philosophy and the basics of how best to organize things. Thanks again so much!! \$\endgroup\$
    – saltcod
    Oct 21 '18 at 23:30
  • \$\begingroup\$ Is this a sensible place to ask follow-up questions? Your answer to my "Does structuring this code as a class makes sense here?" was: "No. You keep no reference to it, thus there is no need for an interface. Only the onScroll function need access to the element and that can be done with closure." I don't really know what this means. Also, I think this part of your answer is related: "You define an object makeSticky and give it 13 exposed (public) properties." Is it bad that I make 13 properties public? I assume so, but I'm not really sure why, philosophically. Any tips? \$\endgroup\$
    – saltcod
    Oct 21 '18 at 23:35
  • \$\begingroup\$ @saltcod Q1 yes the comment are the place to ask follow-ups, but the space is limited. Q2 The last 4 lines of your JS where you create the objectnew makeSticky(... for each ".child" you assign it to the const stick (referencing the new object) That reference is held only inside that function meaning you have no way to access each sticky after you create it. This is not bad, as you don't need to, the onScroll event is the only code that need to. Learning to program means dealing with bugs, more code, more bugs. less code less bugs. (and other benefits) ... continue next comment \$\endgroup\$
    – Blindman67
    Oct 22 '18 at 0:45
  • \$\begingroup\$ @saltcod One way to reduce code complexity is coding only what is needed, nothing more. 80% your code can be dropped if you use closure to hold the state. Closure is an important part of JS and not an easy thing to understand (Not an easy thing to explain), so investing some time learning what it is and how to use it will go a long way ... continue next comment \$\endgroup\$
    – Blindman67
    Oct 22 '18 at 0:47
  • \$\begingroup\$ @saltcod I mentioned the 13 properties to make a point, but there is no min or max, good or bad amount, there are only too many when it can be done with less (in this case none). The core point of the review is about code complexity, You had 47 lines, way too much for the task. Anyways learn closure and keep code simple. The full philosophically answer is way beyond the scope of the comments, nor is there just one way... \$\endgroup\$
    – Blindman67
    Oct 22 '18 at 0:49

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.