Micromodal icon indicating copy to clipboard operation
Micromodal copied to clipboard

Just strange line in the source code

Open DimaGashko opened this issue 4 years ago • 1 comments

I've just found one line in the source code that made me confused

https://github.com/ghosh/Micromodal/blob/master/lib/src/index.js#L106

const body = document.querySelector('body')

Why would you use querySelector to find the body element? There's document.body to do this - shorter, faster, full support

DimaGashko avatar May 28 '20 20:05 DimaGashko

Ok, I don't like that function at all. Too complicated to do so simple thing. I would write so (

/** @param {'enable'|'disable'} value */
scrollBehaviour(value) {
   if (!this.config.disableScroll) return

   if (value === 'enable') {
      document.body.style.overflow = '';
   } else if (value === 'disable') {
      document.body.style.overflow = 'hidden';
   }
}
Or:
/** @param {'enable'|'disable'} value */
scrollBehaviour(value) {
   if (!this.config.disableScroll) return
   const overflow = (value === 'disable') ? 'hidden' : '';
   document.body.style.overflow = overflow;
}
Or:
/** @param {boolean} value */
function toggleScrolling(value) {
   if (!this.config.disableScroll) return
   document.body.style.overflow = (!value) ? 'hidden' : '';
}

DimaGashko avatar May 28 '20 20:05 DimaGashko