-
Notifications
You must be signed in to change notification settings - Fork 4k
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): make duration
prop more advanced
#1967
Conversation
@@ -5,7 +5,7 @@ const transitions = ['jiggle', 'flash', 'shake', 'pulse', 'tada', 'bounce'] | |||
|
|||
const options = transitions.map(name => ({ key: name, text: name, value: name })) | |||
|
|||
export default class TransitionExampleStaticExplorer extends Component { | |||
export default class TransitionExampleTransitionExplorer extends Component { |
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.
Name example's class as file
@@ -1,11 +1,10 @@ | |||
import React from 'react' | |||
import { Message } from 'semantic-ui-react' |
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.
Sort imports.
import { Message } from 'semantic-ui-react' | ||
|
||
const TransitionTypesExamples = () => ( | ||
const TransitionExplorersExamples = () => ( |
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.
Typo.
Codecov Report
@@ Coverage Diff @@
## master #1967 +/- ##
=========================================
+ Coverage 99.8% 99.8% +<.01%
=========================================
Files 148 148
Lines 2580 2584 +4
=========================================
+ Hits 2575 2579 +4
Misses 5 5
Continue to review full report at Codecov.
|
@@ -25,7 +31,13 @@ export default class Transition extends Component { | |||
children: PropTypes.element.isRequired, | |||
|
|||
/** Duration of the CSS transition animation in milliseconds. */ | |||
duration: PropTypes.number, | |||
duration: PropTypes.oneOfType([ | |||
PropTypes.number, |
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.
Looks like we technically also support a string here.
@@ -15,7 +15,7 @@ export interface TransitionProps { | |||
children?: React.ReactNode; | |||
|
|||
/** Duration of the CSS transition animation in milliseconds. */ | |||
duration?: number; | |||
duration?: number | TransitionPropDuration; |
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.
String is also supported, correct?
@@ -14,7 +16,7 @@ export interface TransitionGroupProps { | |||
children?: React.ReactNode; | |||
|
|||
/** Duration of the CSS transition animation in milliseconds. */ | |||
duration?: number; | |||
duration?: number | TransitionPropDuration; |
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.
Also string I believe.
@@ -31,7 +31,13 @@ export default class TransitionGroup extends React.Component { | |||
children: PropTypes.node, | |||
|
|||
/** Duration of the CSS transition animation in milliseconds. */ | |||
duration: PropTypes.number, | |||
duration: PropTypes.oneOfType([ | |||
PropTypes.number, |
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.
Finally, let's add string as a supported type here too.
5f9cbc0
to
f511e0e
Compare
I've rebased to the latest master and added string duration propType / typings support. Will merge on pass. |
Released in |
This PR makes possible to define diffent durations for
hide
andshow
actions.