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

Cap opacity in [0..1] (i get "opacity: 01.0123") #7

Open
joelambert opened this issue Aug 1, 2011 · 14 comments
Open

Cap opacity in [0..1] (i get "opacity: 01.0123") #7

joelambert opened this issue Aug 1, 2011 · 14 comments

Comments

@joelambert
Copy link
Owner

via @madrobby (https://twitter.com/#!/thomasfuchs/status/97955171006550018)

@joelambert
Copy link
Owner Author

This may have been fixed in the latest code in GitHub? If not perhaps this is something that ought to be pushed down into Shifty rather than handled in the Morf.js code?

@jeremyckahn see here - when Morf pulls the opacity value out of the DOM, it returns a string. This was causing an issue before I checked for numerics but perhaps Shifty ought to do this instead? Also perhaps Shifty ought to be capping opacity.

If you disagree I'll add some code into Morf to test this

@jeremyckahn
Copy link
Contributor

@joelambert, I was thinking about this a little bit, and I certainly could make a Mifty/Shifty extension that would take care of this for you. However, the simplest way to solve this might be to use JavaScript's toFixed method to control the precision in the Morf code. I will play with this a little when I have some time in order to figure out the best approach.

@jeremyckahn
Copy link
Contributor

I've been playing around with this issue. I don't want to do type checking in Shifty because that inherently gives more responsibility to Shifty. The library is meant to be low-level and as fast as possible. To do that, it makes several "unsafe" assumptions, such as expecting a Number value instead of a String. The idea is to is to reduce absolutely all unnecessary overhead.

What I'm saying, in my own roundabout way, is that I think that the Number/String type checking should be done in Morf, and a dedicated Shifty extension is overkill.

However, I do think that having simple a way of capping values as you described would be a great addition to Shifty. So, I'm building an extension for that, called "Clamp." The source is here, and an example is here. It seems to work pretty well so far, and once I write a README for it, I will merge it to master.

Do you think that will solve the opacity issue that @madrobby found?

@jeremyckahn
Copy link
Contributor

Wrote a README.

@joelambert Please review the extension, see if it works for you, and let me know. If it does, I'll merge it to master and put it into a Mifty build for you. Thanks!

EDIT: You can just paste the shifty.clamp.js code anywhere after Shifty and it should work for your testing.

@jeremyckahn
Copy link
Contributor

Made some progress on clamp tonight. I added static versions of the clamp methods, but the extension needs some more time in the oven. I should have it ready for you in a few days.

@joelambert
Copy link
Owner Author

Great thanks, I think I'll release without the patch and then do an update next week with clamp included.

@jeremyckahn
Copy link
Contributor

Ok, I have completed work on the Clamp extension and rolled it into Shifty master. I also made a branch called Mifty, which is where I will maintain the Morf-specific build. The file you'll be most interested in is mifty.min.js. It's simply a custom build of Shifty, with only the interpolate, clamp and formula extensions included. Try it out and let me know how it works.

@joelambert
Copy link
Owner Author

Just got round to testing this and Mifty seems to be throwing an error, I get

TypeError: 'undefined' is not an object (evaluating 'this._tweenParams.data') in mifty.js:727 any time I try to initialise a transition from Morf.js.

Any thoughts?

@jeremyckahn
Copy link
Contributor

Hmm. That's strange, I will look at it as soon as I get a chance.

@jeremyckahn
Copy link
Contributor

Ok then, this one was tricky. Long story short, there was a context (what this was referencing) inconsistency with Shifty and the way that it handles filters. I basically fixed it with a null check, and everything seems to be working fine in both Shifty's code and in Morf. Please try out this version and let me know that it works for you.

However, the way that Webkit handles the -webkit-transform Object is... interesting. -webkit-transform has 20-odd properties of its own, apparently, and it has a custom toString() method. From what I can tell, this overridden toString() method is what lets Morf do its magic.

I bring this up, because now Morf is spitting out different CSS than I remember. It's much more verbose now, and I think it's because of the generic nature of how the Clamp and Interpolate extensions work (which I'd really like to avoid changing). The good news is that the actual animation seems to be working just as they did before I added the Clamp code.

So basically, everything works. I just don't know what the long-term implications of this different CSS output is. I'd like you to check that out and tell me what you think. I'm betting that a simple tweak in the Morf code is all that it take to bring the CSS output more in line with what it was.

Soooo... just grab the new Mifty build I linked to, plug it in, see how it works, check the CSS output, and let me know what you think.

Hooray!

@joelambert
Copy link
Owner Author

Thanks, I've tested the code and for me it appears to output the same CSS as before. Can you give me an example of it outputting more verbose CSS?

@jeremyckahn
Copy link
Contributor

Hmm, seems to working for me now. Maybe I threw in a strange line somewhere, or I was just tired. Oh well.

Have you had a chance to play with the Clamp functionality yet?

@jeremyckahn
Copy link
Contributor

Do you have a test case to reproduce this issue? Looks like Shifty's clamp functionality isn't being used by Morf. Is this still an issue?

@joelambert
Copy link
Owner Author

Sorry I've just been totally swamped and not had chance to look at it since I last wrote. I'll make sure I properly test it this week and hopefully roll out a new version of Morf

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

No branches or pull requests

2 participants