-
-
Notifications
You must be signed in to change notification settings - Fork 261
Control background color with css instead of class #429
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
Control background color with css instead of class #429
Conversation
|
看下 CI |
|
OK,我会尽快修复 CI 问题 |
* remove `activeKey` and `defaultActiveKey` api * add preformance.tsx demo for preformance * remove or rewrite some test cases
|
@afc163 ok 了,看看 |
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
===========================================
+ Coverage 99.84% 100.00% +0.15%
===========================================
Files 25 25
Lines 652 628 -24
Branches 169 162 -7
===========================================
- Hits 651 628 -23
+ Misses 1 0 -1
Continue to review full report at Codecov.
|
|
有些代码覆盖率问题,请等我加些测试代码 |
|
修改已完成,本地代码测试和覆盖率测试均已通过 |
| focusMenuElement?: HTMLElement, | ||
| offset: number = 1, | ||
| ) { | ||
| // Key on the menu item will not get validate parent container |
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.
remove unreachable codes
|
覆盖率还是不够啊 |
|
好的,我在排查下 |
|
@MadCcc 好了 |
|
给一个性能分析? |
|
@afc163 抱歉,没做过这方面,想问下性能分析我该以怎样的形式给你?是直接在评论里写更改前后的性能对比? |
|
直接写就好了。 |
|
性能分析:
PS. 组件过多会造成初始渲染卡顿,不过这是另一个话题 |
|
@MadCcc 继续跟进吧 |
|
|
||
| // Active control | ||
| activeKey?: string; | ||
| defaultActiveFirst?: boolean; |
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.
不能删除 API
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.
既然不能删除这两个 API 那我暂时想不到其他的方案,因为组件过多时确实是 node class 增删行为造成的卡顿。
不过可以通过这个 PR (#426),对 parseItems 方法进行缓存也能有效提升一倍的性能,可以复制以下代码进行测试:
// 在我的电脑,未使用 useMemo 缓存时, count = 130 会有明显卡顿,使用后 count = 260 才会有明显卡顿
const count = 260
export default () => (
<div style={{ width: 200 }}>
<Menu
defaultSelectedKeys={['0', '0 - 0']}
>
{new Array(count).fill(0).map((m, i) => {
const title = 'item - ' + i
return (
<SubMenu key={i} title={title}>
{new Array(count).fill(0).map((n, index) => {
const key = i + ' - ' + index
const subTitle = title + ' - ' + index
return (
<MenuItem key={key}>{subTitle}</MenuItem>
)
})}
</SubMenu>
)
})}
</Menu>
</div>
);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.
嗯,现在提供了 items,准备下个大版本去掉语法糖的用法。解析 Node 太过昂贵,也缺少优化的空间。
性能优化:当存在上百个组件时,优化 node class 增删行为造成的卡顿问题
相关问题链接:ant-design/ant-design#32681
解决方案:
activeKey和defaultActiveFirstAPI, 这两个 API 的作用只是给元素设置背景色TODO:在非 inline 模式下,通过键盘操作时,子菜单隐藏时,父菜单背景色会闪烁,原因在与 tryFocus 和 triggerAccessibilityOpen 方法不同步