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

fix(usePagination): fix not using initialPage as a baseline when watc… #41

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

OrdinarySF
Copy link
Contributor

…h changes and changing isLastPage status

@OrdinarySF
Copy link
Contributor Author

在自定义initialPage的值时,会导致isLastPage的值变化不正确;在状监听筛选条件时也会导致类似的问题。

@OrdinarySF
Copy link
Contributor Author

预加载缓存时也会有同样的问题。

@OrdinarySF
Copy link
Contributor Author

OrdinarySF commented Nov 23, 2023

这是关于#38 的补充提交 @JOU-amjs

Copy link
Contributor

@JOU-amjs JOU-amjs left a comment

Choose a reason for hiding this comment

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

还是非常感谢你的贡献🤞😁,可以对代码review里的问题稍微解释一下吗?
如果能加上对应的单元测试用例就更好了,usePagination的单元测试分别在packages\scene-vue\test\usePagination.spec.jspackages\scene-react\test\functions.spec.tsx都有,可以同步添加一下

@@ -162,7 +162,7 @@ export default function (

const pageCountVal = _$(pageCount);
const exceedPageCount = pageCountVal
? preloadPage > pageCountVal
? preloadPage - initialPage + 1 > pageCountVal
Copy link
Contributor

Choose a reason for hiding this comment

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

这里没看懂为什么需要preloadPage - initialPage + 1,我的理解是preloadPage就是预加载的页数了,应该不再需要通过initialPage换算了

@@ -196,7 +196,8 @@ export default function (
const pageVal = _$(page);
const pageCountVal = _$(pageCount);
const dataLen = isArray(statesDataVal) ? len(statesDataVal) : 0;
return pageCountVal ? pageVal >= pageCountVal : dataLen < _$(pageSize);
//Calculate length:currentIndex - startIndex + 1
return pageCountVal ? pageVal - initialPage + 1 >= pageCountVal : dataLen < _$(pageSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

这边也没看懂,pageVal也是当前页的页码了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这两个地方这样写的原因是一样的,统一回复一下:我们可以把它看成一个数组,pageVal代表当前数组的下标,现在已知的是数组的第一个下标为initialPage,则计算当前数组的实际位置(以0为基准)的公式为:pageVal- initialPage+ 1。

这里求的是当前下标是否是最后一位,所以如果通过当前数组下标和元素总个数对比,无法获得其实际是否已经到达最后一个元素,需要获得其当前实际位置,并与总个数-1进行对比才能得到当前下标是否实际到数组末尾。

这里我画了一个白板,稍微有点抽象但是可以参考一下:https://wbd.ms/share/v2/aHR0cHM6Ly93aGl0ZWJvYXJkLm1pY3Jvc29mdC5jb20vYXBpL3YxLjAvd2hpdGVib2FyZHMvcmVkZWVtLzJkYzk1YWJmODU5NDQ1ZGZiMTQ1MTI4ZTIxOWU1ZjNkX0JCQTcxNzYyLTEyRTAtNDJFMS1CMzI0LTVCMTMxRjQyNEUzRF80NDQ5NmU4OC02OTM2LTRiNTktODA3Ny0yMDY5MzZhOWU5YmY=

image

Copy link
Contributor

@JOU-amjs JOU-amjs Nov 27, 2023

Choose a reason for hiding this comment

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

可能这边的理解是不正确的,是否预加载下一页的判断标准就是基于当前页码+1的,而initialPage其实只是用来初始化page变量的,之后的计算直接使用page就可以了,例如当initialPage为2,那么pageVal的初始值也是2,此时我们只需要判断第3页是否可预加载就可以了,如果按你的公式3-2+1 = 2去判断第2页是否有数据错误的。你可以再理下看看是这样的吗

@OrdinarySF
Copy link
Contributor Author

我没有学习过JS领域的单元测试框架,如果有时间我之后会尝试学习并编写一些单元测试,在这之前还需要协助。 @JOU-amjs

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.

2 participants