-
Notifications
You must be signed in to change notification settings - Fork 9
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
React16 #12
React16 #12
Conversation
src/Album.jsx
Outdated
constructor(props) { | ||
super(props); | ||
this.state = { | ||
open: false, | ||
current: props.current, | ||
left: 0, | ||
top: 0, | ||
lastIndex: 0, |
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.
这个 lastIndex 应该也是用 props.current 来生成,这样才可以避免,mount 时候走两遍逻辑。
src/Album.jsx
Outdated
} | ||
}; | ||
|
||
export default polyfill(Album); |
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.
按照 https://github.com/reactjs/react-lifecycles-compat 标准用法,应该 polyfill(Album); 再 export default Album 更靠谱一些。
src/Carousel.jsx
Outdated
this.state = { | ||
left: 0, | ||
top: 0, | ||
lastIndex: 0, |
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.
这里同上,lastIndex 作为 current 的 mirror,应该在 constructor 中也用 current 来生成。
src/Viewer.jsx
Outdated
scale: 1, | ||
current: props.current, | ||
lastOpenStatus: false, |
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.
此处同上。
src/Carousel.jsx
Outdated
@@ -180,3 +190,5 @@ export default class Carousel extends React.Component { | |||
); | |||
} | |||
} | |||
|
|||
export default polyfill(Carousel); |
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.
polyfill 同上。
src/Viewer.jsx
Outdated
@@ -265,3 +276,5 @@ Viewer.propTypes = { | |||
}; | |||
|
|||
Viewer.displayName = 'Viewer'; | |||
|
|||
export default polyfill(Viewer); |
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.
polyfill 同上。
没啥问题了,可以和 文肇 商量下,看谁来发。 |
reopen pr for merging |
update react 16