dream-num / univer

Univer is an open-source alternative to Google Sheets, Slides, and Docs
https://univer.ai
Apache License 2.0
6.34k stars 538 forks source link

[Bug] props transfer of floating dom elements #2753

Closed hxguo closed 1 month ago

hxguo commented 1 month ago

在您提交此问题之前,您是否检查了以下内容?

受影响的包和版本

0.2.2

复现链接

在@univerjs/sheets-drawing-ui的SheetCanvasFloatDomManagerService类中向浮动dom传递props时,props值取自_domLayerMap。 但是_domLayerMap只有在经过addFloatDomToPosition方法时才有赋值过程。这就导致如果是手动添加图层,即有经过addFloatDomToPosition方法时_domLayerMap才有值;如果是从快照中加载的,是没有经过addFloatDomToPosition方法,_domLayerMap是没有值的,从快照加载的最终时进入_drawingAddListener(),如下图: image 图中_drawingAddListener()内只有直接从_domLayerMap取props,实际上是空的,此时props应当可从param中获取(param是来自SHEET_DRAWING_PLUGIN插件快照的数据)。

预期行为

在@univerjs/sheets-drawing-ui的SheetCanvasFloatDomManagerService类中浮动dom的props在从快照中加载时,应当可以传递之前的props。之前的props可以种param中获取

实际行为

在从快照中加载浮动dom时,props无法传递

运行环境

Chrome

系统信息

System: OS: Windows 11 10.0.22000 CPU: (12) x64 11th Gen Intel(R) Core(TM) i5-11400 @ 2.60GHz Memory: 15.98 GB / 31.77 GB Binaries: Node: 20.14.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD npm: 9.6.2 - C:\Program Files\nodejs\npm.CMD pnpm: 9.1.4 - ~\AppData\Roaming\npm\pnpm.CMD Browsers: Edge: Spartan (44.22000.120.0), Chromium (114.0.1823.43) Internet Explorer: 11.0.22000.120

univer-bot[bot] commented 1 month ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Origin Title: [Bug] 浮动dom元素的props传递

Title: [Bug] props transfer of floating dom elements


Before you submitted this question, did you check the following?

Affected packages and versions

0.2.2

Recurrence link

When passing props to the floating dom in the SheetCanvasFloatDomManagerService class of @univerjs/sheets-drawing-ui, the props value is taken from _domLayerMap. However, _domLayerMap only has an assignment process when it passes the addFloatDomToPosition method. This results in that if the layer is added manually, _domLayerMap will have a value only when the addFloatDomToPosition method is passed; if it is loaded from a snapshot, and the addFloatDomToPosition method is not passed, _domLayerMap will have no value and will be entered at the end of loading from the snapshot. drawingAddListener(), as shown below: image In the figure, _drawingAddListener() only takes props directly from _domLayerMap, which is actually empty. At this time, props should be obtained from param (param is the data from the SHEET_DRAWING_PLUGIN plug-in snapshot).

Expected behavior

The props of the floating dom in the SheetCanvasFloatDomManagerService class of @univerjs/sheets-drawing-ui should be able to pass the previous props when loading from the snapshot. Previous props can be obtained from param

Actual behavior

props cannot be passed when loading floating dom from snapshot

Running environment

Chrome

system message

System: OS: Windows 11 10.0.22000 CPU: (12) x64 11th Gen Intel(R) Core(TM) i5-11400 @ 2.60GHz Memory: 15.98 GB / 31.77 GB Binaries: Node: 20.14.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD npm: 9.6.2 - C:\Program Files\nodejs\npm.CMD pnpm: 9.1.4 - ~\AppData\Roaming\npm\pnpm.CMD Browsers: Edge: Spartan (44.22000.120.0), Chromium (114.0.1823.43) Internet Explorer: 11.0.22000.120

jikkai commented 1 month ago

Could you plz help confirm this issue @weird94

weird94 commented 1 month ago

fixed on https://github.com/dream-num/univer/pull/2791/files#diff-dd163e96ae5b499d0357cba6841497d7954d57f00ecb98df0231b1f24a8aee3eR515 In case that we don't store props internally, please use SheetCanvasFloatDomManagerService.addHook to register a hook to get props from outside.

我们在内部无法持久话存储props, 所以需要用户自行注册SheetCanvasFloatDomManagerService.addHook以从外部获得float-dom的props

version >= 0.2.4

univer-bot[bot] commented 1 month ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

fixed on https://github.com/dream-num/univer/pull/2791/files#diff-dd163e96ae5b499d0357cba6841497d7954d57f00ecb98df0231b1f24a8aee3eR515 In case that we don't store props internally, please use SheetCanvasFloatDomManagerService.addHook to register a hook to get props from outside.

We cannot store props persistently internally, so users need to register SheetCanvasFloatDomManagerService.addHook to obtain float-dom props from the outside.

version >= 0.2.4

hxguo commented 1 month ago

好的,等版本发布后我试一下。 原本是想可以从param中获取,因为我有把props存进快照里,从快照加载时会带有props信息。

univer-bot[bot] commented 1 month ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Okay, I'll give it a try after the version is released. Originally I thought I could get it from param, because I have saved the props in the snapshot, and the props information will be included when loading from the snapshot.

hxguo commented 1 month ago

另外有一个小建议,期望addFloatDomToPosition方法返回的drawingId可以由外部传进去,而不是内部直接生成一个随机ID,随机ID可以是外部没传的情况下生成。这个ID会用做悬浮层内容容器的id,在业务上有比较重要的意义。 image

univer-bot[bot] commented 1 month ago

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

In addition, there is a small suggestion. It is expected that the drawingId returned by the addFloatDomToPosition method can be passed in from the outside, instead of directly generating a random ID internally. The random ID can be generated without external transmission. This ID will be used as the ID of the content container of the suspension layer, which has important business significance. image

jikkai commented 1 month ago

另外有一个小建议,期望addFloatDomToPosition方法返回的drawingId可以由外部传进去,而不是内部直接生成一个随机ID,随机ID可以是外部没传的情况下生成。这个ID会用做悬浮层内容容器的id,在业务上有比较重要的意义。 image

good catch! Let me create a new issue for it.