-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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(Transition): add component #1435
Conversation
❤️ |
Main problem that I have there it's missing I need to pull |
Is there anything we can do to help get this completed and merged? My team is excited to have transitions. |
@bjmiller Thanks for ping. This PR is one of the most complicated places, it needs enough time to find the best solution, since this is not my first attempt to solve it 😄 I hope to find enough time on this weekend to create MVP which meets all the requirements |
I've pulled code from react-transition-group#24 as start point. I will try to spend all free time to finish it. TODO
|
Codecov Report
@@ Coverage Diff @@
## master #1435 +/- ##
==========================================
+ Coverage 99.75% 99.76% +<.01%
==========================================
Files 145 147 +2
Lines 2477 2577 +100
==========================================
+ Hits 2471 2571 +100
Misses 6 6
Continue to review full report at Codecov.
|
0da3649
to
9f6c1dd
Compare
e7c5625
to
19a988f
Compare
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.
@levithomason I'm finally ready to say that we are ready to review 🎉
To all: this component is ready for some user testing and feedback.
How?
If you could pull this branch feat/transition-2
, npm install
, npm start
, and navigate to http://localhost:8080/modules/transition, you'll see the Transition component examples. Please have a look at the code, play with the editors, and let me know what you think.
Sweet! I will get some testing in on this branch this weekend. Great work @layershifter. I was sad when the old version had to be abandoned, but I am glad we are using the non-deprecated package that React team is supporting with this one. |
Just wanted to drop by and say that y'all are MVPs for going through the legwork of porting Semantic-UI. Particularly here when it gets really messy, hooking into React's animation systems. I've been using React for years and that stuff still mystifies me. Thanks for working on this! I'll try to test it if my team finishes required UI soon, and if not, I look forward to using it once it's merged. Thank you for contributing to OSS ❤️ |
<Transition.Group | ||
as={List} | ||
duration={1000} | ||
divided size='huge' |
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.
Whoops, two lines: divided size='huge'
/** Primary content. */ | ||
children?: React.ReactNode; | ||
|
||
/** Duration of the CSS transition animation in microseconds. */ |
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.
microseconds
=> milliseconds
src/modules/Transition/Transition.js
Outdated
/** Primary content. */ | ||
children: PropTypes.node, | ||
|
||
/** Duration of the CSS transition animation in microseconds. */ |
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.
microseconds
=> milliseconds
/** Primary content. */ | ||
children?: React.ReactNode; | ||
|
||
/** Duration of the CSS transition animation in microseconds. */ |
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.
microseconds
=> milliseconds
/** Primary content. */ | ||
children: PropTypes.node, | ||
|
||
/** Duration of the CSS transition animation in microseconds. */ |
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.
microseconds
=> milliseconds
Heads up, I'm going to push some refactors and updates here for a bit. |
cfb5e41
to
925efe2
Compare
type='range' | ||
value={duration} | ||
/> | ||
<Form.Button content={visible ? 'Hide' : 'Show'} onClick={this.handleVisibility} /> |
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.
When changing state during a transition the example does not behave correctly as the transition is interrupted. I'd like to disable the form controls while the transition is in progress.
In trying to disable/enable the Show/Hide button in the explorer examples, I now think we need to fire the callbacks on Transition.Group
for each transition as well. I'm not sure there is a way to capture start/complete without this.
Would you mind updating the examples to show how this is possible?
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.
Hm, thanks for attracting attention there. I'm that Transition.Group
should also have onComplete
, onHide
, onShow
, onStart
and this should be illustrated in separate 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.
However, I want to do this in sepate PR, this one is already big enough.
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.
Separate PR sounds good.
This is looking fantastic, thanks! I have a couple questions in addition to what I left inline. It would be great to get your thoughts behind these items: Transition vs Transition.GroupI'm a little confused on the use of Intuitively, I would expect a https://semantic-ui.com/modules/transition.html#/examples Could you give some info on what the differences are and why two components are needed to accomplish this?
|
9816f61
to
053b9de
Compare
a242223
to
7aebe8b
Compare
7aebe8b
to
97f77a9
Compare
I've pushed some changes here, no changes to the logic, only naming and docs:
In the next PR(s) let's address:
|
@levithomason: Is there an ETA for a new release that includes this feature? |
Released in |
I do the best I can to make a release every weekend. |
Is there a way to animate Accordion or shall we expect it in later releases? @levithomason |
@rutvijprimaseller Currently there is no support for animating in accordions. This @layershifter you might know... are there separate tasks set aside for converting each of these yet? |
No, there are no separate tasks. But, plans for them exist of course. I want begin from |
@layershifter That makes my day to hear. |
can I track the status of Accordion transitions somewhere? It's too fast, so I want a slightly slower animation. |
Is it possible to run static animation transitions in a loop? Like a continuous tada animation on a call icon for an incoming call notification until it's clicked on and picked up. |
@subodhpareek18 I don't see why not |
Fixes #200.
This PR is second try for
Transition
component. It usesreact-addons-transition-group
for deal with SUI's CSS animations.