Refactor to leverage ES6 language features and a more modular object model

[joelvh]

This refactor maintains the same public API, while updating the internals.

• Refactor string concatenation to string interpolation
• Remove global variables and instead manage state on a simple object for clarity and flexibility
• Add helper methods to consolidate common tasks
• Simplify some logic for clarity

@alyssaxuu this is a subsequent update following #37

[joelvh]

@alyssaxuu I would recommend merging #37 first. Once that's done, the diff for this PR will more clearly show what's been refactored. I'm also making separate commits for each change so the refactor can be tracked in stages.

I will continue to refactor the internals when I get some more time, so please feel free to merge this as a start.

[joelvh]

@alyssaxuu this can be merged as a refactor of the codebase. The goal here was to DRY up the code and use method names to better indicate what is going on. I'll have more refactoring as a result of the React (#39) version.

Any feedback to get this merged?

[cliffordfajardo]

@joelvh - just an onlooker with some feedback 👀 .

The demo code should remain at the root as it was before.
Maybe in a different separate PR after this, you can propose moving demo in src, but I think it's best in the demo folder. src to me means the actual source code of the library.

The new dot files should be in a separate PR.
I like the addition of the dotfiles. I know exactly what they do and I know they don't impact the JS source code in terms of functionality. However, these should also be put in a separate PR to make this code review much simpler to digest. As a reviewer, there are too many moving parts in this PR know.

The CSS files should have remained untouched (for this PR)
From a maintainer's point of view, for the most part I was only expecting the Javascript code changes Optimizations could be done in a separate PR and can be viewed in isolation.

Also having these dotfiles in a separate PR gives folks the opportunity to give feedback much more easily, just for these changes.

• .editorconfig
• .gitignore
• .npmignore - the content from this file can just be put in .gitignore, there's no need for this 😄

The introduction of the package.json and should be in a separate PR.
We're introducing new tooling and those types of changes tend to be better in isolation, also it gives room for discussion around the individual dependencies themselves.

Because we're introducing babel, I think it warrants a discussion on a few things with @alyssaxuu about her thoughts on:

• how far back we want to support browsers. Maybe she wants this to be an evergreen javascript library for modern browsers? I think more thoughtful discussion in general as a group about babel settings, micro-bundle, parcel-bunder are needed.
• jest was introduced but it's not used.

Overall I like javascript code changes! There are just too many changes and moving parts in this single PR. My suggestions are:

• make JS changes only
• introduce bundlers/transpilation in a future PR in isolation to discuss things like what settings, dependencies @alyssaxuu or any other onlookers 👀 think.
• try to keep file names as they were

[joelvh]

Hi @cliffordfajardo thanks for the feedback!

• The demo code was meant to live in src and then there’s a build command to generate the demo in the root, which currently is in .gitignore, but could commit that build artifact.
• can put the dotfiles in a separate PR
• the CSS changed because of the auto formatting defined in dotfiles 😐.
• I setup Convert to NPM package #37 as the initial PR to setup NPM first.

Are you a maintainer if this project?

[cliffordfajardo]

@joelvh - thanks for the clarifications

are you a maintainer?

Nope, just someone who recently started playing around with this awesome javascript library. I left comments with the intent of helping improve the chance of this type of commit making it through since there were a lot of changes

[joelvh]

Unfortunately, there's no interest in refactoring the code base to make it easier to understand and more modular. Maybe in the future.