fix: 兼容不设置高度时,窗口变化后更新画布尺寸 #1900
Conversation
|
|
你好，这个功能都做了什么测试呢？是怎么测试的 |
那个 issue 的 codesandbox 我没打开。我现在的理解，如果 resize 未生效，先确认 resize 后，graphModel 中的 width/height 是否变成了预期的值。1. 如果值变了，容器样式没有变，那根据 width、height 值更新容器样式即可；2. 如果值没有变，先定位问题(为什么没变)，再按 1 的逻辑处理。 |
|
@boyongjiong 我目前想到的能够兼容不配置宽或者高度时重新resize及配置时不清空resize，加入了两个属性记录。 |
|
是这样的，提交一下吧，我看一下。建议后续 PR 也是提供测试的代码，就跟你现在做的这个一样 |
|
效果如下： |
|
刚刚review发现， |
|
@boyongjiong 请问为什么关闭这个 pr 呢，即使可能不是最优解，但是我认为目前 resize 方法现在是无效的，如果不兼容 resize 监听，也需要移除 resize 监听事件吧。 |
| @@ -56,7 +56,7 @@ class Graph extends Component<IGraphProps> { | |||
| this.props.options.width = this.props.graphModel.width | |||
| this.props.options.height = this.props.graphModel.height | |||
| } | |||
| private throttleResize = () => throttle(this.handleResize, 200) | |||
There was a problem hiding this comment.
这个位置的节流没生效，代码有点小问题，如果生效了的话。只补充style，视口尺寸变了的话。style会被重置。
|
|
||
| this.width = options.width || this.rootEl.getBoundingClientRect().width | ||
| this.height = options.height || this.rootEl.getBoundingClientRect().height | ||
| if (options.width) { |
There was a problem hiding this comment.
我突然发现这个写法的问题，如果我把 options.width 设置成 0，是不是就去拿容器的大小了
这段的代码是不是可以写成下面这种，当 options.width 是 undefined 时，取 container 宽度
// 初始化宽度 + 设置是否使用容器宽度标志位
this.width = options.width ?? this.rootEl.getBoundingClientRect().width
this.isContainerWidth = isNil(options.width)
// 高度同上
this.height = options.height ?? this.rootEl.getBoundingClientRect().height
this.isContainerHeight = isNil(options.height)
| @@ -1411,8 +1432,20 @@ export class GraphModel { | |||
| * 重新设置画布的宽高 | |||
| */ | |||
| @action resize(width?: number, height?: number): void { | |||
| this.width = width || this.rootEl.getBoundingClientRect().width | |||
| this.height = height || this.rootEl.getBoundingClientRect().height | |||
| if (width) { | |||
There was a problem hiding this comment.
这段代码是否可以同样简单写
// 更新宽度 + 更新标志信息
this.width = width ?? this.rootEl.getBoundingClientRect().width
this.isContainerWidth = isNil(width)
this.height = height ?? this.rootEl.getBoundingClientRect().height
this.isContainerHeight = isNil(height)
| @@ -39,9 +39,24 @@ type ContainerStyle = { | |||
| @observer | |||
| class Graph extends Component<IGraphProps> { | |||
| private handleResize = () => { | |||
There was a problem hiding this comment.
这段逻辑我有些没看太懂的点：
- 是只有当画布是容器的宽或高时，才更新画布吗？
- 为什么需要重新设置 options.width、options.height，这个是用户外部传入的，是否有必要再更新呢？它是不是只在初始化时用一次
另外，就是代码的问题：代码中 通过 this.props.graphModel 读取值的频率太高了，可以提前解构出来下面用
const { graphModel, options } = this.props
const { width, height, isContainerWidth, isContainerHeight } = graphModel
let nextWidth: number | undefined = width
let nextHeight: number | undefined = height
const needUpdate: boolean = isContainerWidth || isContainerHeight
if (isContainerWidth) {
nextWidth = undefined
}
if (isContainerHeight) {
nextHeight = undefined
}
if (needUpdate) {
graphModel.resize(nextWidth, nextHeight)
}
options.width = graphModel.width
options.height = graphModel.height
There was a problem hiding this comment.
1.没错，考虑到如果用户设置了宽高，那么视口变化时就不需要重复更新尺寸了
2.关于重新设置option的宽高，我这边考虑时外部能拿到这个和获取这个属性，我在测试用例里面也打印了这个测试，所以给他重新更新为新的属性。如果不太需要的话可以移除。
最后优化部分：感谢提醒，已优化为解构方式
| lf.graphModel.isContainerHeight, | ||
| lf.graphModel.isContainerWidth, | ||
| ) | ||
| console.log('current option size', lf.options.width, lf.options.height) |
There was a problem hiding this comment.
此处可以获取到options里面的宽高，所以进行了更新
|
本 PR 代码跟现在 master 上代码已经有冲突了，建议拉一下最新代码，然后基于 master rebase ，更新一下当前 pr 代码哈 |
|
你的操作好像有点问题 😂，在你的分支，基于原项目 master 分支 rebase 之后，在你的分支 |
|
git reflog 撤回一下你在我评论之后的操作，重新整一下 |
感谢，这次呢？ |
好了哈，代码已合并，感谢 PR~ 欢迎持续光临哈哈哈 |
解决resize传递宽高后渲染无效，并兼容窗口变化事件。