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

React16 #3

Merged
merged 7 commits into from
Jun 28, 2018
Merged

React16 #3

merged 7 commits into from
Jun 28, 2018

Conversation

AppianZ
Copy link
Collaborator

@AppianZ AppianZ commented Jun 27, 2018

No description provided.

// 等待数字位置复原完毕,
// 开始设置完整的数字
if (this.props.count !== prevProps.count) {
this.setState({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在 didUpdate 里不能 setState,这样会引起二次渲染,所有的 setState 相关的操作,都考虑放到getDerivedStateFromProps 中

@@ -77,9 +82,9 @@ class ScrollNumber extends React.Component {
return 10 + num;
}
const currentDigit = getNumberArray(this.state.count)[i];
const lastDigit = getNumberArray(this.lastCount)[i];
const lastDigit = getNumberArray(this.state.lastCount)[i];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我感觉 lastCount 的逻辑很奇怪,如果放到 getDerivedStateFromProps 里,因为 gDSFP 在每次渲染时都会调用,要看下跟以前在 cWRP 里的表现还是否一致。根据代码里的注释。

@coveralls
Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage decreased (-0.8%) to 94.958% when pulling d47d498 on AppianZ:react16 into 5254017 on uxcore:master.

// 等待数字位置复原完毕,
// 开始设置完整的数字
if (this.props.count !== prevProps.count) {
this.lastCount = prevProps.count;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里逻辑不太对,以前是拿的 state.count ,那这里应该指向 prevState.count 吧

@AppianZ AppianZ merged commit bac6af9 into uxcore:master Jun 28, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants