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

jQuery-ification #6

Closed
wants to merge 2 commits into from
Closed

Conversation

RobertLowe
Copy link

Adds data-purplecoat-easing
Adds data-purplecoat-duration
Adds onComplete event for fade
Adds jQuery options
Adds docs
Adds basic reactivity to plugin (changing data property will apply changes on click)
Adds as an author
Moves data-purplecoat-color to face element
Moves style.css to index.html demo/docs
Renames purplecoat-min.js to standard purplecoat.min.js

Uh, enjoy

Adds `data-purplecoat-easing`
Adds `data-purplecoat-duration`
Adds `onComplete` event for fade
Adds jQuery options
Adds docs
Adds basic reactivity to plugin (changing data property will apply changes on click)
Adds <Robert Lowe> as an author
Moves `data-purplecoat-color` to face element
Moves `style.css` to `index.html` demo/docs
Renames `purplecoat-min.js` to standard `purplecoat.min.js`
…into gh-pages

Conflicts:
	bower.json
	purplecoat-min.js
	purplecoat.js
@RobertLowe
Copy link
Author

@ellekasai

Let me explain why I did this.

I converted the library into a jQuery as a part of standard-ification, and also to fix potential bugs and non potential bugs, I thought it would be a good learning example as well.


The first problem I wanted to correct was the bulked up styles, seen here:

https://github.com/ellekasai/purplecoat.js/blob/gh-pages/purplecoat.js#L3-L7

This is a problem for readability as well as it's completely static and unchangeable for users.

Which is mostly fixed by a plugin options here:

https://github.com/RobertLowe/purplecoat.js/blob/gh-pages/purplecoat.js#L7-L35


The next thing I wanted to fix was the ability to have changes applied when you update data attributes, fixed here:

https://github.com/RobertLowe/purplecoat.js/blob/gh-pages/purplecoat.js#L79-L95


Another change I wanted to apply was moving all the data attributes (except toggle) to the relative element. Such as data-purplecoat-color. Seen here:

https://github.com/ellekasai/purplecoat.js/blob/gh-pages/purplecoat.js#L21


I also wanted to fix deeply nested code as part of good practises:

https://github.com/ellekasai/purplecoat.js/blob/gh-pages/purplecoat.js#L44-L51

I tried to keep every under 3-4 tabs, as deeply nested functions can become cumbersome.


To do most of these changes and to standardize things I morphed the code into a jQuery plugin, which allowed for configuration, reinitialization.

I saw this on hacker news and wanted to spend a bit of time making a good example.

While I understand it can look more complex, there is plenty of documentation, I tried to provide a comment per line. Feel free to question me about changes.

Anywho, cheers and happy hacking.

@ellekasai
Copy link
Owner

Thank you so much for your pull request, and I'm sorry for my late reply. I've been very busy for the past few days since I released purplecoat.js. And I'll be attending startup week vancouver (http://www.startupweekvancouver.ca/) full time until 11/21. I promise to take a look at your pull request closely after that's over. Hope you don't mind!

@RobertLowe
Copy link
Author

@ellekasai I don't mind at all, it's your project after all.

Have fun at startup weekend, I've been to those as well, they're good fun. Focus on learning and networking. Don't push yourself too hard with ideas (all nighters and such).

Have fun!

@ellekasai
Copy link
Owner

Hi @RobertLowe,

Thanks so much again for your contribution. After some thinking, I decided to keep this library as simple as possible. I did implement a jQuery API, but it's still very simple. http://ellekasai.github.io/purplecoat.js/#jquery-api I probably will not make it more complicated than this unless a lot of people ask for a particular feature.

Also, based on your suggestion, I made it possible to add data-purplecoat-color to the target element. Thanks for your suggestion!
https://github.com/ellekasai/purplecoat.js/pull/15/files

I also wrote CONTRIBUTING.md to let future contributors know about my decision.

But I did mention your name on README.md for special thanks:
https://github.com/ellekasai/purplecoat.js

@ellekasai ellekasai closed this Dec 15, 2014
@ellekasai ellekasai self-assigned this Dec 15, 2014
@RobertLowe
Copy link
Author

No worries, hope you learned something.

Cheers

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