-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(Ellipsis): Handle some post-update issues #6797
base: master
Are you sure you want to change the base?
Conversation
@@ -66,12 +60,18 @@ export default function useMeasure( | |||
const fullMeasureHeight = fullMeasureRef.current?.offsetHeight || 0 | |||
const singleRowMeasureHeight = | |||
singleRowMeasureRef.current?.offsetHeight || 0 | |||
const rowMeasureHeight = singleRowMeasureHeight * rows | |||
const rowMeasureHeight = singleRowMeasureHeight * (rows + 0.5) |
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.
能直接 +0.5 而不改 setWalkingIndexes 不,理论上启动阶段就是应该和 startMeasure 同步的
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.
理论上是这样同步更新值,但是实际运行时,在线demo运行的结果却是不符合预期的。
从代码执行层面来说,其实只有状态为MEASURE_WALKING时才需要walkingIndexes的值,所以在设置MEASURE_WALKING状态前去更新walkingIndexes应该也是合理的。且这样做也能解决了结果不符合预期的问题。
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.
理论上是这样同步更新值,但是实际运行时,在线demo运行的结果却是不符合预期的。
那直接包一个 unstable_batchedUpdates
告知其同时启动就行了
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6797 +/- ##
==========================================
- Coverage 92.72% 92.70% -0.02%
==========================================
Files 335 335
Lines 7198 7198
Branches 1804 1803 -1
==========================================
- Hits 6674 6673 -1
- Misses 489 517 +28
+ Partials 35 8 -27 ☔ View full report in Codecov by Sentry. |
关于第一个文本省略字数错误的问题
我分析了一下,其实是setWalkingIndexes设置的值最后被赋值导致
第一次调用正常计算,然后第二次由useResizeEffect触发,先setStatus然后setWalkingIndexes,进入useLayoutEffect计算PREPARE时,此时walkingIndexes的值还是上一次的值,然后进入MEASURE_WALKING,执行了一次diff就等于0,然后renderContent,这时setWalkingIndexes初始化设置的值生效了,因为state是STABLE_ELLIPSIS,然后初始化的值变成渲染了一半的文本,所以文本省略看起来算的有问题,实则被覆盖了。
不过这个问题也很奇怪,为什么setWalkingIndexes没有与setStatus一同生效? 而且只有在codesandbox链接里这个简单页面渲染下100%复现这个问题,其他正常业务场景和官网文档实例中写一个一样的代码都没这个问题,难绷。
现在做法是把setWalkingIndexes放在setStatus(MEASURE_WALKING)前面,这样就不会有这个问题了,而且其实只有MEASURE_WALKING时才需要walkingIndexes的值。
引申优化问题
这里其实还有一个重复计算的问题,当fullMeasureRef大于rowMeasureHeight时才会计算,但是这个fullMeasureRef永远都是渲染content所有内容的高度,导致issue中debug截屏看到的一直是59>40。所以即使文本超出被截断了,下一次进入判断的时候也会重新命中,然后计算截断。
之前组件的实现就没有这个问题,因为下一次判断时container的内容就是已经处理过的,然后在取值container.offsetHeight,就会<=maxHeight,从而跳过判断。
我根据之前的思路再结合现在的实现,在计算前先获取containerRef的高度,然后取最小值,这样就解决了截断情况下fullMeasureRef一直大于rowMeasureHeight的情况,正常的跳过了。
但是引发了一个新的问题,跳过后state变成了STABLE_NO_ELLIPSIS,然后又渲染了content,导致元素触发ResizeObserver重新计算,结果就是state一直在STABLE_ELLIPSIS与STABLE_NO_ELLIPSIS反复横跳死循环了。
目前能想到两个解法