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

Extract hover state by creating a global state #95

Merged
merged 1 commit into from
Nov 2, 2016
Merged

Extract hover state by creating a global state #95

merged 1 commit into from
Nov 2, 2016

Conversation

yesmeck
Copy link
Member

@yesmeck yesmeck commented Oct 31, 2016

写了个类似 redux 的 store,把 hover 状态提了出来,这样 hover 状态变化的时候就不需要渲染整个 Table 了。

ant-design/ant-design#3096

@coveralls
Copy link

Coverage Status

Coverage increased (+1.07%) to 83.234% when pulling 6f0f3bd on yesmeck:optimize-hover into ea2dc4b on react-component:master.

@afc163
Copy link
Member

afc163 commented Oct 31, 2016

是不是只要在 shouldComponentUpdate 里判断,只有 currentHoverKey 的变化,不触发 render。效果是一样的?

@yesmeck
Copy link
Member Author

yesmeck commented Oct 31, 2016

应该是可以的,不过刚刚发现,现在的 shouldComponentUpdate 也有问题,因为固定列的 columns 每次都会重新生成 https://github.com/react-component/table/blob/master/src/Table.jsx#L386 所以固定列的 shouldComponentUpdate 其实是没用的,props.columns 每次都是新的。

我先看看能不能把这个 shouldComponentUpdate 优化下。

@afc163
Copy link
Member

afc163 commented Oct 31, 2016

或者用 context 解决。

@afc163
Copy link
Member

afc163 commented Oct 31, 2016

columns 目测有两个地方可以优化:

  1. getLeftFixedTablegetRightFixedTable 里,不要生成 fixedColumns,而是把原始的 columnsfixed 直接传给 TableRow ,在 TableRow 里进行过滤。

    getLeftFixedTable() {

  2. getLeafColumns 里应该判断,如果没有 children 不需要生成新的数组。

    const leafColumns = this.getLeafColumns(columns || props.columns);

const listeners = [];

function setState(partial) {
state = assign({}, state, partial);
Copy link
Member

Choose a reason for hiding this comment

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

直接用 { ...xx }

@yesmeck
Copy link
Member Author

yesmeck commented Oct 31, 2016

还有 groupColumns 也会生成新的 columns。

@yesmeck
Copy link
Member Author

yesmeck commented Oct 31, 2016

我想写一个专门来管理各种 columns 计算的类,然后把计算结果都缓存起来。

@yesmeck
Copy link
Member Author

yesmeck commented Nov 1, 2016

或者用 context 解决。

context 怎么搞?

@yesmeck
Copy link
Member Author

yesmeck commented Nov 1, 2016

我重新发个 PR。

@yesmeck yesmeck closed this Nov 1, 2016
@yesmeck yesmeck reopened this Nov 1, 2016
@yesmeck yesmeck changed the title Extract hover state by creating a global state [WIP]Extract hover state by creating a global state Nov 1, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 81.038% when pulling 6f0f3bd on yesmeck:optimize-hover into ea2dc4b on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.4%) to 83.696% when pulling a02297a on yesmeck:optimize-hover into e79fe28 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 83.152% when pulling 6c67987 on yesmeck:optimize-hover into e79fe28 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 83.152% when pulling 24c1a9d on yesmeck:optimize-hover into e79fe28 on react-component:master.

@yesmeck yesmeck changed the title [WIP]Extract hover state by creating a global state Extract hover state by creating a global state Nov 2, 2016
@yesmeck
Copy link
Member Author

yesmeck commented Nov 2, 2016

@afc163 这个 PR 也调整过了。

@coveralls
Copy link

Coverage Status

Coverage increased (+1.6%) to 83.842% when pulling b2a9e82 on yesmeck:optimize-hover into e79fe28 on react-component:master.

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