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

Bring up to date, make it work. #103

Closed
wants to merge 5 commits into from
Closed

Bring up to date, make it work. #103

wants to merge 5 commits into from

Conversation

jonscottclark
Copy link
Contributor

This bundled PR addresses multiple issues:

  • This plugin now works in a browserify / CommonJS setting.
  • Syntax errors in package.json have been fixed.
  • Fix a ReferenceError that gets thrown when checking for an undefined value (see the end of the original file)
  • When the instance is destroyed, the .is-truncated class will be removed. (when a plugin is removed it should reset the markup to its original state).

These are all changes I made in order to get this plugin working in my browserify setup. Hopefully whoever is seeing these pull requests can merge and ensure that the large number of users looking to use this in 2016 can still find some use in it. Thanks!

@jonscottclark jonscottclark changed the title Bring up to date. Bring up to date, make it work. Feb 29, 2016
@jameswilson
Copy link

Patch looks good. One question, does the AMD/CJS compatibility require the switch from \t indentations to double-space indentation? If not, seems like that should be a separate issue/request.

@jonscottclark
Copy link
Contributor Author

You're right, the indentation change was totally opinionated. I've added another commit to go back to the original indentation.

@jonscottclark
Copy link
Contributor Author

Going to separate these into multiple PRs.. getting really polluted now and won't be able to easily merge others' PRs if this goes through.

@jonscottclark
Copy link
Contributor Author

FWIW, there are places where patches have been inserted where there are tabs instead of spaces, and double quotes instead of single quotes, presumably because the maintainers aren't preparing releases... so I just went ahead and did a wholesale beautify on the file for my own sanity.

@jameswilson
Copy link

sanity++ And I agree, that I usually code with 2 spaces, not tabs, but I also know that the jQuery Style Guide dictates indentation with tabs, and lots of (unnecessary) filler whitespace around many language constructs. Oh well.

@jonscottclark
Copy link
Contributor Author

Yeah, oh well.

The recent PR merge-fest seemed to lack any consideration, so I had to step back, for sanity, again. Maybe I'll try and submit more PRs to clean it up some more.. but not prioritizing it 😭

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