-
Notifications
You must be signed in to change notification settings - Fork 442
feat: v2 #169
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
Conversation
breaking change: see notes
|
This is wonderful, I just noticed that tinycolor was actually quite large, go check it out and 15 hours ago this lands :) Thanks, looking forward to the release! |
|
Oh - I mistook the closing for merging 😞 |
|
@wmertens it can still be used/found at https://github.com/TypeCtrl/tinycolor however you lose the advantage of a popular repo with lots of people finding whatever bugs exist. |
|
Not much point in a popular repo where found bugs are then not actually fixed :( @bgrins I totally understand time pressures/burnout etc, but would it not be better to get some help maintaining the repo? I (and I'm sure others) would gladly do some PR merging to assist… |
|
@scttcper Sorry for dropping the ball on this - especially after you took the time to do the updates to get this landable. I started to review it, but with the rewrite and API changes I didn't finish and it fell off my radar. If you could commit to helping out with maintaining the repo, I will get this landed. I do have one specific question around the API methods that have moved out (random, readability, etc) - how can those be referenced from a bundle? That is, for a CDN linked JS file or one copied out through dist/ and loaded as a |
|
@bgrins there's still a browser specific bundle created with if you run build and then load Edit: no worries btw. Open source isn't an obligation to work on a project forever 😺 |
OK, tested that locally with an HTML file and a script pointing to that and indeed it works just as before! Just to confirm: the umd min module exposes an API which would require migration from: a) As for (a), I have a couple of questions around backwards-compat with v1:
|
It looks like that could be handled with TypeScript static properties. We could also export the functions directly to the public API for non UMD packages (as is currently done) for convenience. |
|
One more question around backwards-compat: |
|
@bgrins i've added a default export so the following will continue to work as it does now. const tinycolor = require('tinycolor2');
const x = tinycolor('red');
x.toHex() // "ff0000"to use it as a class it is still const TinyColor = require('tinycolor2').TinyColor;
// or
import { TinyColor } from 'tinycolor2';
const x = new TinyColor('red');
x.toHex() // "ff0000"
Can go any way you want on this one. |
…files. use only /dist folder
|
I think I'm still seeing some issue with the way |
|
OK, now if I pull down https://github.com/bgrins/TinyColor/tree/v2 locally, here are a couple of notes:
Maybe a dep is missing?
|
|
Sweet! I've been watching this PR for a while and I'm excited to see that it made it through. |
I'd be generally inclined to export it as lower-cased so that the v1->v2 migration is smoother (and the migration docs are easier to write). But I guess it's a balance between that vs following class naming conventions and how much of a pain changing it back at this point will be. |
Is GH_TOKEN also only needed for npm release, or does travis need it for something else as well? |
|
GH_TOKEN is used to push a release to the github release tab w/ git tag that matches the version published on npm. example I feel like if they're going to switch to using class with the on the demo, its |
That sounds fine to me, assuming we can support the static methods on the class as mentioned in (3) in #169 (comment). |
A working build can be seen/installed at the hard fork at https://github.com/TypeCtrl/tinycolor
updated docs: https://typectrl.github.io/tinycolor/
api docs: https://typectrl.github.io/tinycolor/docs/
breaking changes: see notes
on travis this would need a few new tokens
npm token createused with semantic release to publish from CI. If you want to publish manually i can remove this.notes
sideEffectstinycoloris now exported as a class calledTinyColorrandom, an implementation of randomColor by David Merfield that returns a TinyColor object, the old one is still available aslegacyRandomTinyColor.<function>. See updated docs for use examples.readability,fromRatiomoved outrandommoved out and renamed tolegacyRandomtoFilterhas been moved out and renamed totoMsFiltermix,equalsuse the current TinyColor object as the first parametertint()andshade()tinycolor PR Add tint() and shade() #159isValid,formatare now properties instead of functionscloses #151