-
Notifications
You must be signed in to change notification settings - Fork 11
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
Feat:: Shape Transition Effect:: Adding a demo section13 with the Shape transition effect #72
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! Some comments.
<!-- Temp: library for smooth easing --> | ||
<script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/gsap.min.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rather not use GSAP, but it's ok to leave it to me to later replace it with native code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yes Like I said that was only to have a smooth transition but feel free to make your own in a RAF using https://github.com/AndrewRayCode/easing-utils/blob/master/src/easing.js We could remove it
demo/shape-transition.js
Outdated
resiveHandler(); | ||
}); | ||
|
||
const resiveHandler = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const resiveHandler = () => { | |
const resizeHandler = () => { |
demo/shape-transition.js
Outdated
fade.resolution = [target.offsetWidth, target.offsetHeight]; | ||
}; | ||
|
||
window.addEventListener('resize', resiveHandler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
window.addEventListener('resize', resiveHandler); | |
window.addEventListener('resize', resizeHandler); |
demo/shape-transition.js
Outdated
instance.play(); | ||
resiveHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instance.play(); | |
resiveHandler(); | |
resizeHandler(); | |
instance.play(); |
demo/shape-transition.js
Outdated
}; | ||
|
||
window.addEventListener('resize', resiveHandler); | ||
resiveHandler(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is not needed
set to(media) { | ||
this.textures[0].data = media; | ||
}, | ||
set resolution([width, height]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have a utility for this, though we don't yet have a way to enforce it or require it automatically. So not sure what's best here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that I can import the resolution utility in demo/shape-transition.js
. But I can't import it in src/transitions/shape.js
. To me it would make more sense to use it in src/transitions/shape.js
directly and merge it to the other uniforms but not sure how to do that with the current implementation. So I'm not using it for now but would be nice in the future, let me know if you see an easy solution 👍🏻
u_resolution: 'vec2', | ||
u_nbDivider: 'float', | ||
u_shapeBorder: 'float', | ||
u_shape: 'float', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need shape
as a uniform. Probably would be better off as a string parameter of the constructor, that we put directly into the template of the shader. Can be enum of 'circle'
, 'diamond'
, 'square'
.
It's possible to have these set on the exported function, like we did with enums in displacement
effect for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some uniforms as params of the shape function. I used ENUM, it's a good idea to have a cleaner approach of what are the differents param possibilities for shape, effect and direction.
Adding a demo section 13 with the new shape transition effect.
Notes
shape.js
transitions in the transitions folder, in can be called like so:shape-transition.js
in the demo folder file to show a demo of it.Questions
Is there a reason why it's using iframes to show the different demos?
I got several issues because of that: I couldn't see my logs showing in Chrome because they come from inside an iframe. Hopefully the logs are showing on Firefox so I could debug on that.
I got some cache clear issues, disabling cache when the console is open seems to work most of the time.
It's not convenient to import common functions/utilities through different demos.
My suggestion would be to use different html files (1 for each demo) with their own js final build. That's possible in rollup or vite using the input configuration. That's what I did for : https://robin-wix-xp.vercel.app/circles-transition/ / https://robin-wix-xp.vercel.app/grid-offset/ (I'm using vite but it's using rollup under the hood)
Not a priority of course. I need to move on on other effects and using iframes for demos is working for me now, but might be useful in the future.
Things to implement in Wix (I guess)
The effect is ready but it's using the default params by default. I think ideally for the user they will be able to use some of the GUI/DEFAULT parameter. In that case you can change this:
into this:
So each parameter can be set up like this:
Screen.Recording.2025-02-09.at.18.19.56.mp4