Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update build system & fix #22 #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update build system & fix #22 #30

wants to merge 3 commits into from

Conversation

Toanzzz
Copy link

@Toanzzz Toanzzz commented Jun 9, 2018

First off, thank you for such amazing library. I've been using this lib for some small project, but the bug in #22 are so annoying that I decided to fix it myself. So here we are:

[JS]

[CSS]

  • Switch to PostCSS and Autoprefixer
  • Add missing "!" in copyright comment, to prevent that comment be tripout

(That were a lot of changes, sorry. I just want to help you upgrade the code to newer, modern style and don't mean to offend anyone. Please let me know what you think about this PR 馃槃)

[JS]
- Using webpack, babel to build & bundle js target UMD library
- Rewrite code in ES6 and CommonJS style
[CSS]
- Switch to PostCSS and Autoprefixer
- Add missing "!" in copyright comment, to prevent that comment be tripout
@thinker3197
Copy link
Owner

This is a much-needed PR @Toanzzz. Thanks for your time and effort!
Give me a day or two and I'll merge this after reviewing changes.

@Toanzzz
Copy link
Author

Toanzzz commented Jun 10, 2018

That was a quick reply thinkker3197. Sorry that I kind of take it my way and add so many things. If you think any changes are unnecessary, please feel free to edit them (and I'll appreciated if you let me know how to improve it)

@dallington
Copy link

@Toanzzz First, thanks for the correction, and I'm sorry for the way of writing, I'm Brazilian and I'm using google translator.

I believe I encountered an error on line 212. SecondaryObject did not exist. I added the following code to verify.

for (const prop in primaryObject) {
聽聽聽聽 o [prop] = (secondaryObject! = null && secondaryObject.hasOwnProperty (prop))? secondaryObject [prop]: primaryObject [prop]
聽聽 }

@Toanzzz
Copy link
Author

Toanzzz commented Dec 1, 2018

@dallington that extend function that cause the error is the original one that I extracted and move to bottom. I checked and it only used once when initializing the settings. I believe we can use ES6's spread operator for this kind of thing:

// Original
self._settings = extend(self._settings, options)

// ES6
self._settings = { ...self._settings, ...options}

With this we don't have to check anything :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants