HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Simple modals in vanilla JS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
vanillasimplemodals

Problem

Recently I was looking for some really simple modal/popup solution in vanilla JS, but nothing popped. So I wrote mine component. It works great so far, have all the options I need, but I don't know if I can improve the code itself.

JS:

``
/* A very simple modal component
You can close the modal either with
esc key, hitting the "Close" button
or simply clicking anywhere on the page except the said modal.
Apart from this, you can open your modal with url using it's id as a hash,
eg: http://localhost#modal/loginModal
*/

import HelpMe from '../helpers/helpers';
const __ = new HelpMe();

export default class Modal {
constructor() {
require('./modal.scss');

this.modals = document.querySelectorAll('modal-window');
this.classNames = 'modal';
this.activeClassName = 'modal--active';

this.modalTriggers = document.querySelectorAll('[modal-trigger]');

this.format();

__.onEvents([window], 'load hashchange', () => { this.listenForHash(); });
}

format() {
__.forEach(this.modals, (modal) => {
const title = modal.getAttribute('title');

modal.classList.add(this.classNames);

modal.setAttribute('id', __.slug(title)); / Created ID /

this.createHTML(modal, title) / Creating inner html /
});
}

wrap(item, title, content) {
item.innerHTML =



${title}
${content}

;
}

createHTML(modal, title) {
const inner = modal.innerHTML; / Get current content /
this.wrap(modal, title, inner);

this.listenForClick();
this.listenForClose();
}

setHistory(id = null) {
if (id) {
window.history.pushState(null, null,
${document.location.origin}#modal/${id}`);
} else {
window.history.pushState(null, null, document.location.origin);
}
}

open(modal) {
this.close();

document.getElementById(modal).classList.add(this.activeClassName);
this.setHistory(modal);
}

close(modal = null) {
if (modal) {
document.getElementById(modal).clas

Solution

import HelpMe from '../helpers/helpers';
const __ = new HelpMe();


This doesn't tell me what HelpMe is unless I look for it. Same can be said for __. We don't know what it is. It's not like jQuery or Underscore where they have been known for quite a while to use $ and _, respectively. Best if you just named it Helper.

__.forEach


Not sure why you need this. array.forEach already exists in ES5. I assume this is for looping through array-like structures. Most array-like structures can be turned into a proper array with Array.prototype.slice.

export default class Modal {


This is misleading. Modal is a class and the name is singular. But it looks like what it does is collect all modal HTML and launch all of them with methods. It behaves like... a singleton. You can either:

  • Have Modal handle just one modal per instance, based on constructor configuration.



Expanding a bit on point #1, you export a class that holds a state of a single modal. The idea is that your modal instance knows about just one modal setup. It's your typical OOP stuff, object holding config and state for a single instance, and has methods to manipulate that.

export default function Modal(config){
  // stuff
}

// Usage

import Modal from 'path/to/modal/module';

var modal = new Modal({ element: '#modal-template' });

modal.open();
modal.close();


  • Export the module with global modal handling functions instead of a class.



Expanding a bit on point #2, the idea is that your module will not export an instantiatable class. Instead, all the module will have are a collection of functions. This operates much closely to how I see your code does it (call function, give directions, operate based on it), but throws away the idea of classes and one instance working on multiple modals.

export function createModal(){/* creates an object that holds a modal */}

export function openModal(modal){/* accept modal, open it */}

export function closeModal(modal){/* accept modal, close it */}

// Usage
import ModalFunctions from 'path/to/modal/module';

var modal = ModalFunctions.createModal({/* config */});
openModal(modal);
closeModal(modal);


The module isn't exporting a class and you operate it like you o. It's just a bunch of functions that create and handle modals.

open(modal) {
  this.close();

  document.getElementById(modal).classList.add(this.activeClassName);
  this.setHistory(modal);
}


Instead of fetching the elements on every call, why not store references to the DOM elements when you create the Modal instance? This way, you can avoid the overhead of DOM fetching.

this.close(button.parentNode.parentNode.getAttribute('id'));


You are hardcoding the structure of your traversal to 2 ancestors up. You are restricting the flexibility of your modal markup. Consider using delegation, attaching the handlers on the top-most element of your modal and listen for descendant element events.

transform: translate(-50%, calc(-50vh - 200%));


A neat trick to center modals is to create 2 nested divs. The outer one would have display:table. The inner one will have display:table-cell. Set the vertical-align of an element inside that inner div to middle and margin to 0 auto to center it both horizontally and vertically.

Code Snippets

import HelpMe from '../helpers/helpers';
const __ = new HelpMe();
export default class Modal {
export default function Modal(config){
  // stuff
}

// Usage

import Modal from 'path/to/modal/module';

var modal = new Modal({ element: '#modal-template' });

modal.open();
modal.close();
export function createModal(){/* creates an object that holds a modal */}

export function openModal(modal){/* accept modal, open it */}

export function closeModal(modal){/* accept modal, close it */}

// Usage
import ModalFunctions from 'path/to/modal/module';

var modal = ModalFunctions.createModal({/* config */});
openModal(modal);
closeModal(modal);
open(modal) {
  this.close();

  document.getElementById(modal).classList.add(this.activeClassName);
  this.setHistory(modal);
}

Context

StackExchange Code Review Q#128595, answer score: 3

Revisions (0)

No revisions yet.