Skip to content

Instantly share code, notes, and snippets.

@Jahans3
Last active July 18, 2017 15:45
Show Gist options
  • Save Jahans3/2706fdf2828743d5361b2bf015195089 to your computer and use it in GitHub Desktop.
Save Jahans3/2706fdf2828743d5361b2bf015195089 to your computer and use it in GitHub Desktop.
import * as app from '../../../app';
import defaults from './preloader.json';
/**
* Preloader
*
* @access public
*
* @param {(String|HTMLElement|NodeList)} els
* @param {Object} custom
*/
// Avoid function keyword if you aren't using 'this' inside
// Consider simulating named parameters with parameter destructuring
// Named params add an extra layer of safety (no specific order needed and param symbols must match)
// export preloader = ({ els = 'preloader', custom }) => {
// This should probably be a default export as the only export in the module
// Also if it's the default export you can omit the symbol
// export default function (els = 'preloader', custom) { ...
export function preloader(els = 'preloader', custom) {
// Don't mutate parameters
// const preloadedCum = app.custom('preloader', custom);
custom = app.custom('preloader', custom);
// Consider destructuring Synergy, custom and config in the import
// import { Synergy, custom, config } from '../../../app';
// It's convention that you only capitalise variable names if it's a class, devs will assume Synergy is a class
app.Synergy(els, (el, options) => {
// You need to have a way of removing event listeners
// No need for the wrapping function exports.hide is exactly the same
window.addEventListener('load', () => exports.hide());
// This will throw an error if options is undefined
app.Synergy(options.name).component('close').forEach(trigger => {
// Need a way of removing these event listeners
// No need for the wrapping function again
trigger.addEventListener('click', () => exports.hide());
});
// I'm not sure what exports is here, but the usual way to export submodules is with the export and export default keywords
// The following export: export const show = () => { ... }
// Would allow you do do: import { show } from './module'; show();
// If that's what you want
exports.show = () => exports.toggle('show');
exports.hide = () => exports.toggle('hide');
// Also if exports is something custom then you're implicitly declaring a global in this module
// If the scope can only live inside this function declare a const and return that
// A boolean param named show would make it clearer what is happening here
// Named parameters on modifier fn would make this clearer
// Consider determining whether or not the second param should be unset or set prior to calling the fn
// Would make this clearer
// const isSet = (el.modifier('loaded') || operator === 'show') ? 'unset' : 'set'
exports.toggle = operator => el.modifier(
'loaded', (el.modifier('loaded') || operator === 'show') ? 'unset' : 'set'
);
}, defaults, custom);
// If Synergy always accepts your defaults as an argument consider not passing it as a param
// and instead keeping it inside a closure where you defined Synergy
// If you want to set a property on a class/object you should use a setter method which safely modifies the property
// Also tells anyone reading your code that preloader will be set in an unseen part of your app
app.config.preloader = Object.assign(defaults.preloader, custom);
return exports;
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment