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.

Sign up for GitHub

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

Jump to bottom

docs(word-cloud): 添加词云图文档和demo #1571

Merged
merged 3 commits into from Sep 17, 2020
Merged

docs(word-cloud): 添加词云图文档和demo #1571

merged 3 commits into from Sep 17, 2020

Conversation

zhangzhonghe
Copy link
Member

@auto-add-label auto-add-label bot added the documentation Improvements or additions to documentation label Sep 14, 2020
hustcc
hustcc reviewed Sep 14, 2020
View reviewed changes
examples/word-cloud/basic/demo/image-mask.ts
// 宽高设置最好根据 imageMask 做调整
width: 600,
height: 400,
autoFit: false,
Copy link
Member

Choose a reason for hiding this comment

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

autoFit = true 可以运行吗？

Copy link
Member Author

Choose a reason for hiding this comment

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

可以，我改一下

Copy link
Member Author

Choose a reason for hiding this comment

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

autoFit = true和width``height一块设置时会有问题，我现在解决一下

@hustcc
Copy link
Member

hustcc commented Sep 14, 2020

已经 CR，主要添加 autoFit = true 的 demo。

lxfu1
lxfu1 reviewed Sep 15, 2020
View reviewed changes
examples/word-cloud/basic/demo/image-mask.ts
autoFit: false,
wordField: 'name',
weightField: 'value',
imageMask,
Copy link
Member

Choose a reason for hiding this comment

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

不支持 CDN 地址吗？

Copy link
Member Author

Choose a reason for hiding this comment

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

啥意思？是直接指定字符串地址吗，这个不支持，那样的话需要render方法异步执行

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
Member Author

Choose a reason for hiding this comment

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

好的

hustcc
hustcc reviewed Sep 15, 2020
View reviewed changes
src/plots/word-cloud/types.ts
width?: number;
/** 单词所占盒子的高度 */
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
Member Author

Choose a reason for hiding this comment

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

暂时还没有用到

Copy link
Member

Choose a reason for hiding this comment

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

用不到的要不要直接删除。

@hustcc
Copy link
Member

hustcc commented Sep 17, 2020

pr 问题不大，先合并了。存在的问题或者优化，下一个 pr 继续看。

@hustcc hustcc self-requested a review September 17, 2020 04:33
@hustcc hustcc merged commit c9245ee into antvis:master Sep 17, 2020
@zhangzhonghe zhangzhonghe deleted the docs/word-cloud branch September 17, 2020 15:01
@zhangzhonghe zhangzhonghe mentioned this pull request Sep 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lxfu1 lxfu1 lxfu1 left review comments

@hustcc hustcc hustcc approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation PR: merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zhangzhonghe @hustcc @lxfu1