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

fix: 删除默认icon,添加删除逻辑,修复初始化时销毁无法删除图层,添加ts定义,修改基本用法例子 #344

Merged

Conversation

starInEcust
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Dec 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
react-amap ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2023 3:19am

@starInEcust
Copy link
Contributor Author

需要把useLayoutEffect的一起push上来,还是一会儿我再来一个pr

// // 图片相对 position 的锚点,默认为 bottom-center
// anchor: 'center',
// };
// }

Copy link
Member

Choose a reason for hiding this comment

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

移除了默认 icon,不会显示空白?

Copy link
Member

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.

这里上次讨论的不太深入。首先官方给的demo里是没有icon的。
我这边用的也是没icon的,就是纯文字,显示子市场。
所以我认为这个东西默认应该是没icon的,我也在给到的demo里更新了示例。
还有一个关键点就是即便是我有icon,也不会去用这里给到的默认icon,必然会让UI切一个。
你要是觉得得默认icon这里我就改回去- -
image

Copy link
Member

Choose a reason for hiding this comment

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

@starInEcust 😯,我们的应用场景是默认有个 icon ,需要兼容老的版本

可以修改这一句

const { visiable, children, text, icon = initIcon, ...other } = props;

这样兼容老的使用,不会影响你的使用,当没有传递 icon 的显示 默认 initIcon, icon=null 的时候就可以默认不显示图标

@jaywcjlove
Copy link
Member

@starInEcust 已经修改了 icon ,我们在更新的时候原则上不破坏过去的 API,如果破坏了过去的 API, 我们需要发个大版本

jaywcjlove added a commit that referenced this pull request Dec 5, 2023
github-actions bot pushed a commit that referenced this pull request Dec 5, 2023
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.

None yet

2 participants