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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added package.json and UMD loader wrapper #14

Closed
wants to merge 6 commits into from

Conversation

purtuga
Copy link

@purtuga purtuga commented Dec 9, 2016

Hi Taufik,
I found your color-picker widget a few weeks ago and decided to use it for a few projects that I have. I neede da basic color picker and love that yours (in comparison ot many others outthere) is light in size, dependency free and not bloated with too many extra feature. What I found to be missing (for me :) ) is a package.json, which allows folks to use package managers (ex. npm) and (most important), that it provides support for use with module patterns, so that the glboal window space is not automtically poluted with the CP namespace...

With this contribution, I am providing:

  • A package.json file
  • Changes to color-picker.js to wrap the library in an UMD declaration.

This will enable your library to be used with packge manager, as well as modern module loaders (ex. ES2015 import or AMD or CommonJS.

Thanks for your work on this and for sharing it on GitHub.

/Paul

- Added package.json so that library can be used with package managers like npm
- Wrapped library in Universal Module Definition code, avoiding the use of the global space if
  one of the common modules patterns are used.
- Added package.json so that library can be used with package managers like npm
- Added package.json so that library can be used with package managers like npm
- Wrapped library in Universal Module Definition code, avoiding the use of the global space if
  one of the common modules patterns are used.
- Added package.json so that library can be used with package managers like npm
@taufik-nurrohman
Copy link
Owner

Uhm, the issue already discussed here: #1

Will add this next time in a duplicate file named as color-picker.umd.js or maybe you could just create a fork of my project and add the UMD support so that I can link your modified version to the README file 👍

The package.json is okay, but the UMD pattern is not. I want to keep this library written in plain JavaScript and to be functioned directly as it is. If it is for color picker, then it should function as a color picker, that’t it.

The reasons behind my statement that I will never add any UMD patterns to my projects is because there are no(t yet)? standards on importing and exporting modules in the ECMAScript specification. Yes, I know that AMD and CommonJS are very popular and widely used but they are not part of the JavaScript itself. They are just another JavaScript products, like jQuery. So adding this pattern is like adding support for the jQuery plugin.

standards

Too much competition and choice. The solution is ES6 module syntax.

taufik-nurrohman added a commit that referenced this pull request Dec 10, 2016
@purtuga
Copy link
Author

purtuga commented Dec 10, 2016

Thanks for getting back to me.
I agree that the solution is ES6 Module Pattern... When I first saw your library, I was hoping that I could just import it into my projects (most of them are now written in ES6 module pattern), but it does not seem to implement an export default statement. Everything is just public and global, which is (IMO and experience) never a good think.
ES6 did standardize the module pattern... What is still missing is a standard around the module loaders, and thus the current need for the UMD type of wrapper...

This is no problem.. I will use my fork of you project instead and may also make other changes of issues I'm seeing (ex. your .fit() method does not seem to account for color pickers inside of absolute positioned elements or elements that have offset applied to them).

Thanks again for taking the time getting back to me...

/Paul.

@purtuga purtuga closed this Dec 10, 2016
@taufik-nurrohman
Copy link
Owner

There may also be a need for kind of HSLA color support because the reverse version of HSLA color converter is still buggy in the demo page.

@taufik-nurrohman
Copy link
Owner

taufik-nurrohman commented Dec 10, 2016

Your .fit() method does not seem to account for color pickers inside of absolute positioned elements or elements that have offset applied to them.

Maybe I should define context before offset parameter so that we can set custom boundaries like:

// $.fit([boundaries[, offset]]);
picker.fit(picker.picker.parentNode, [4, 10]);
picker.fit(window, [4, 10]); // default

@purtuga
Copy link
Author

purtuga commented Dec 10, 2016

I have not looked exactly at what the issue is in side of .fit()... My currently solution: I'm using a custom "position()" method on the cpInstance.picker HTML element... I invoke it by placing a listener on the enter event. My position funtion (I use it in several other use cases) uses a combination getBoundingClientRect() + window.body.scrollTop -- which seems to work OK, because the cpInstance.picker is always appended to window.body.

What you suggest might work.. I assume the array is offset from [top, right]?

@taufik-nurrohman
Copy link
Owner

Left and top (X and Y).

https://tovic.github.io/color-picker/#section:method-fit

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

2 participants