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

Zs #40

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

Zs #40

wants to merge 3 commits into from

Conversation

bigint405
Copy link
Contributor

抽取导览接口;利用事件中心初步更改之前的标签事件、导览事件

Copy link
Owner

@ZWboy97 ZWboy97 left a comment

Choose a reason for hiding this comment

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

基本ok,有些重构的效果,有些设计的小问题,你看一下。

@@ -24,9 +24,9 @@
]
],
"hot_spot_list": [
[
Copy link
Owner

Choose a reason for hiding this comment

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

改变了数据结构时候,之前的部分正常嘛?之前这么设计的原因是便于将这个列表直接映射转化未一个Map

Copy link
Owner

Choose a reason for hiding this comment

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

兼容性需要测试一下,会影响之前已经实现的部分吗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

本地测试过,目前没有发现问题

@@ -37,11 +37,24 @@
"img_url": "https://youmucloud.oss-cn-beijing.aliyuncs.com/xr/fuzhou/zhong_shan_she_qu.jpg",
"img_height": 100,
"img_width": 150
},
"event": {
Copy link
Owner

Choose a reason for hiding this comment

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

event不单独提取出来,直接和上边的属性评级,这样如何?

@@ -63,19 +84,36 @@ class SpriteShapeHelper {

setHotSpotList = (hot_spot_list) => {
this.resetHotSpotGroup();
this.hotSpotMap = new Map(hot_spot_list);
this.hotSpotMap = new Map();
this.eventMap = new Map();
Copy link
Owner

Choose a reason for hiding this comment

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

需要将event单独用一个Map进行映射吗?优势是什么??? 将event直接放在hotSpotMap中可以吗?它们都有相同的key,且本来它们就是在一块的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

event单独出来主要是为了和之前的代码兼容,如果合在一起就可能需要更改一些已有的接口,比如添加热点以前是传两个参数:热点属性、事件,现在只需要传热点属性,而需要调用接口的一方提前把事件写在热点属性里面,这样需要修改调用方的代码。当然也可以接口参数不变,在接口内部的时候把事件放在热点属性里面,但是这样会修改用户传进来的热点属性、有可能导致一些奇怪的bug;或者把用户传进来的热点属性复制一份,这样会增加运算量。目前在写把event放在hotSpotMap中的版本,暂时采取不改变对外接口的方法。

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