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: recycler view with overflow error #662
Conversation
wssgcg1213
commented
Sep 7, 2021
•
edited
edited
- FIX display: sliver 与 overflow-y 冲突
- FIX: sliver 节点的滚动事件和 scrollTop/scrollLeft 值处理
- FIX: Sliver 回收节点功能修复
- 允许 launch 传递 showPerformanceOverlay 的开关默认值
- 给 scroll 事件增加 debounce 用来提升性能
- FIX: img 标签如果没有上屏, 循环调用 scheduleFrame 的问题
- FIX: img 标签如果没有上屏, 无法获取到 width/height 的问题 (使用 naturalWidth/naturalHeight)
- 其它代码风格调整
可以补充下 sliver 这些 bad case 相关的测试用例。 |
@@ -136,7 +136,7 @@ nativeDynamicLibrary.lookup<NativeFunction<NativeEvaluateScripts>>('evaluateScri | |||
final DartParseHTML _parseHTML = | |||
nativeDynamicLibrary.lookup<NativeFunction<NativeParseHTML>>('parseHTML').asFunction(); | |||
|
|||
void evaluateScripts(int contextId, String code, String url, int line) { |
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.
launcher 里面 lineOffset 也没用了吧?看起来 lineOffset 定义了 0 传递进去就没了,目前应该没有指定起始位置的需求。
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.
跟 JSC API 对齐参数的原因吧, @andycall 看下还要不要
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 没问题,我说调用的地方也去掉一下。
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.
调用的地方 @andycall 的意思应该是预留的, 给外层传递用的, 放在抽象类里面提供的是默认值
applyStickyChildrenOffset(); | ||
paintFixedChildren(scrollOffset, axisDirection); | ||
|
||
if (eventHandlers.containsKey(EVENT_SCROLL)) { | ||
_fireScrollEvent(); | ||
if (!_shouldConsumeScrollTicker) { |
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.
这样 scroll 事件的触发就会慢一帧,建议针对首帧做特殊处理
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.
我觉得没有必要, 反而增加复杂度了;
渲染本身是异步的, 触发 scroll 是同步于手势采集事件, 跟帧渲染在同一时刻反而从逻辑上保持同步了
if (display == CSSDisplay.sliver) { | ||
_sliverBoxChildManager = ElementSliverBoxChildManager(this); | ||
} else { | ||
_sliverBoxChildManager = null; |
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.
为啥还会有 else,不写不是一样的效果吗?
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.
节点可以被 detach, 再 attach
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.
所以如果在游离状态修改了 display 的属性, 再 attach 就会有问题;
detach(); | ||
} | ||
|
||
detach(); |
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.
在 detach() 方法里面处理是否挂载
的判断, dispose 的语义是释放
事实上 detach 方法里面已经判断过了, 所以这里不判断, 保证生命周期的触发成对
children!.forEach(add); | ||
void addAll(List<RenderBox>? children) {} | ||
|
||
void insertIntoSliver(RenderBox child, { RenderBox? after }) { |
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.
这个方法命名有点看不懂,方法的类是 recycleLayout,insertInto 表示将 recycleLayout 插入 sliver,改成 insertSliverChild 或者 insertChildIntoSliver 好点
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.
LGTM, 我修改一下
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.
sliver 是条带的意思, 所以 child 就是 sliver 本身; intoSliverList or insertSliverChild 才是正确的~
超过 7 天, 触发一个 approve 合并 |