-
-
Notifications
You must be signed in to change notification settings - Fork 50.8k
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
fix: bugs on pageSize change in List #24514
Conversation
感觉这个 LIst 组件原本的测试样例挂的有点多啊... |
|
好像不能单纯这么改,props的pagination变化的时候呢? |
PR 里的 changelog 要填写,不要删。 |
好的 |
这个地方按照以前那个逻辑,如果在props里面的pagination 按照 issue 那样写死了 current 参数,点页码切换的时候也只会停在写死的那个页码那里。这个地方本身的逻辑就有些问题。 |
https://codesandbox.io/s/antd-reproduction-template-52sep?file=/index.js:0-760 具体可以参考这个逻辑 |
ref: #10357 |
|
我的意思是,看能不能处理下pagination的更新,顺手改掉这个问题。 |
好的,这里我晚上回家研究一下~ |
changelog 要面向用户写而非面向开发人员,虽然我也经常不由自主这样干 😂 |
哈哈哈,好的,谢谢提醒 |
@yoyo837 那看看这样可行吗? QUQ |
Codecov Report
@@ Coverage Diff @@
## master #24514 +/- ##
=======================================
Coverage 99.21% 99.21%
=======================================
Files 365 365
Lines 7287 7290 +3
Branches 2035 2037 +2
=======================================
+ Hits 7230 7233 +3
Misses 57 57
Continue to review full report at Codecov.
|
我补一下测试~ |
PTAL |
貌似感觉修错了bug。。。用户反馈的是改变 |
你的意思是 用户需要自行通过 onShowSizeChange 的监听 修改传入的分页信息来达到切换size的效果? |
那这样思考的话,就不是bug了。当前就是对的 |
改变 pageSize 也应该触发 onChange,因为 current page 基本上会变。 |
最终的size 任然由用户通过onChange修改传入值吗? @fireairforce 那就往 onShowSizeChange 没 触发 onChange 的方向改。 |
好的,不好意思,我刚开始可能思考方向想错了,谢谢~ |
可以参考 Table 的处理。 |
但是 |
|
done~ |
补一下用例呗,onSizeChange搭配onChange的 |
好,下班晚上回去补~ |
每次用git rebase upstream/xxx 与主库同步之后,推上来就得 |
done了~ |
PTAL |
rebase了 |
|
||
wrapper.find('.ant-select-selector').simulate('mousedown'); | ||
wrapper.find('.ant-select-item-option').at(1).simulate('click'); | ||
expect(handlePaginationChange).toBeCalled(); |
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.
toHaveBeenCalledWidth
,把参数也写出来。
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.
ok
@@ -106,6 +106,11 @@ function List<T>({ | |||
return (page: number, pageSize: number) => { | |||
setPaginationCurrent(page); | |||
setPaginationSize(pageSize); | |||
if (eventName === 'onShowSizeChange') { | |||
if (pagination) { |
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.
这一行不需要。
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.
类型还有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.
Done~ |
[中文版模板 / Chinese template]
🤔 This is a ...
🔗 Related issue link
fix #24501
📝 Changelog