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

[WIP]List: add virtualized & infinite #7660

Closed
wants to merge 5 commits into from

Conversation

nikogu
Copy link
Contributor

@nikogu nikogu commented Sep 19, 2017

  • infinite
  • virtualized
  • api optimize
  • functional detail optimize

issue:#4905 (comment)
refs:

之前 issue 里回复的方案,今天尝试了一下,行不通,主要原因是因为每次都要去计算所有元素的位置才能够知道哪些显示哪些不显示(即使存储了也是一样的,因为需要在滚动的时候判断位置),这样的话相当耗费性能,所以这也是其它所有 virtual list 都要固定高度的原因,不过还是在思考当中,如果有了更好的方案,会及时同步。

所以基本上是结合世面上的 virtual,简化了一些,添加了 infinite 功能,合成了 List.Infinite 组件。

底层的 virtual 采用 react-tiny-virtual-list,之所以没有用 react-virtualized,是因为它真的太大太复杂,而且效果有些莫名其妙

<List.InfiniteList
  hasMore={this.state.hasMore}
  loading={this.state.loading}
  onLoad={this.handleInfiniteOnLoad}
  dataSource={this.state.data}
  height={500}
  itemHeight={117}
  renderItem={item => (
    <List.Item>
      <List.Item.Meta
        avatar={<Avatar src="https://zos.alipayobjects.com/rmsportal/ODTLcjxAfvqbxHnVXCYX.png" />}
        title={<a href="https://ant.design">{item.title}</a>}
        description={item.content}
      />
      <div style={{ padding: 24 }}>Content</div>
    </List.Item>
  )}
/>

@codecov
Copy link

codecov bot commented Sep 19, 2017

Codecov Report

Merging #7660 into antd-3.0 will increase coverage by 0.48%.
The diff coverage is 46.37%.

Impacted file tree graph

@@             Coverage Diff              @@
##           antd-3.0    #7660      +/-   ##
============================================
+ Coverage      82.1%   82.58%   +0.48%     
============================================
  Files           214      215       +1     
  Lines          4453     4450       -3     
  Branches       1342     1339       -3     
============================================
+ Hits           3656     3675      +19     
+ Misses          797      775      -22
Impacted Files Coverage Δ
components/list/index.tsx 86.2% <100%> (+49.2%) ⬆️
components/list/Infinite.tsx 45.58% <45.58%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe7c85f...a1a921c. Read the comment docs.

const { dataSource, renderItem } = this.props;

return (
<div key={`list-infinite-${index}`} style={style}>
Copy link
Member

Choose a reason for hiding this comment

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

这个 key 会有性能问题吧?

Copy link
Contributor Author

@nikogu nikogu Sep 19, 2017

Choose a reason for hiding this comment

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

不过 react-tiny-virtual-list 本身显示什么的都是按照 index 来的,好像显示不了几个就被 隐藏了

import { List, Card, message } from 'antd';
import { List, message, Avatar } from 'antd';

const InfiniteList = List.Infinite;
Copy link
Member

@afc163 afc163 Sep 19, 2017

Choose a reason for hiding this comment

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

infinite 应该作为 List 的一个 object|boolean 配置,默认为 false。也不需要单独的 renderItem 方法。

Copy link
Contributor Author

@nikogu nikogu Sep 20, 2017

Choose a reason for hiding this comment

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

写在里面如:

<List>
    xxxxx
</List>

virtual 暂时没想到好的实现

之前 issue 里回复的方案,今天尝试了一下,行不通,主要原因是因为每次都要去计算所有元素的位置才能够知道哪些显示哪些不显示(即使存储了也是一样的,因为需要在滚动的时候判断位置),这样的话相当耗费性能,所以这也是其它所有 virtual list 都要固定高度的原因,不过还是在思考当中,如果有了更好的方案,会及时同步。

所以如果是 children,其实很难把控每个 children 具体是什么,高度为多少(渲染了才知道,所以又会涉及上面的问题)

感觉确实需要一个 renderItem

Copy link
Member

Choose a reason for hiding this comment

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

主要是两个组件 API 差不多,写法要改就很麻烦。

loading={this.state.loading}
onLoad={this.handleInfiniteOnLoad}
dataSource={this.state.data}
height={500}
Copy link
Member

Choose a reason for hiding this comment

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

height 必须要配么,这个还蛮费解的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个我尝试下自动计算,其实 react-virtualized 它也会提供一个套件 AutoSizer,可以参考默认添加上去。

@nikogu nikogu changed the title List: add virtualized & infinite [WIP]List: add virtualized & infinite Sep 20, 2017
key = dataSource[rowKey];
} else {
key = dataSource.key;
}
Copy link
Member

Choose a reason for hiding this comment

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

index 兜底。

@nikogu
Copy link
Contributor Author

nikogu commented Sep 21, 2017

突然发现,rn 的 List,都是用的 renderItem,好像也很有道理,我们要不要一起改:https://facebook.github.io/react-native/docs/listview.html
https://facebook.github.io/react-native/docs/flatlist.html

<List
  itemLayout="horizontal"
  showLoadMore
  onLoadMore={() => {}}
  data={}
  renderItem={()=>(
    <List.Item actions={[<a>编辑</a>, <a>更多</a>]}>
      <List.Item.Meta
        avatar={<Avatar src="https://zos.alipayobjects.com/rmsportal/ODTLcjxAfvqbxHnVXCYX.png"/>}
        title={<a href="https://ant.design">Ant design</a>}
        description="Ant Design, a design language for background applications, is refined by Ant UED Team"
      />
      <div style={{ padding: 24 }}>Content</div>
    </List.Item>
)}
/>

这样 virtualized 也好做一点,之前那种,每次也都是写 data.map,对比下来是有点奇怪,因为子元素内容基本都是一样的。 @afc163 怎么看

@afc163
Copy link
Member

afc163 commented Sep 21, 2017

也行,两边 API 统一了比较好。

@nikogu nikogu mentioned this pull request Sep 21, 2017
4 tasks
@nikogu
Copy link
Contributor Author

nikogu commented Sep 21, 2017

#7692

@nikogu nikogu closed this Sep 21, 2017
@nikogu nikogu deleted the virtualized-infinite-list branch September 21, 2017 09:58
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

2 participants