Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

feat(plugin-chart-word-cloud): convert word cloud to use encodable #258

Merged
merged 11 commits into from
Nov 20, 2019

Conversation

kristw
Copy link
Collaborator

@kristw kristw commented Nov 16, 2019

🏆 Enhancements

  • Define WordCloud's API using encodable.

Basic
image

encodes color by word length
image

encodes font family by first letter
image

@kristw kristw requested a review from a team as a code owner November 16, 2019 02:11
@netlify
Copy link

netlify bot commented Nov 16, 2019

Deploy preview for superset-ui-plugins ready!

Built with commit 06b2e61

https://deploy-preview-258--superset-ui-plugins.netlify.com

@codecov
Copy link

codecov bot commented Nov 16, 2019

Codecov Report

Merging #258 into master will decrease coverage by 1.08%.
The diff coverage is 54.54%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #258      +/-   ##
=========================================
- Coverage   37.99%   36.9%   -1.09%     
=========================================
  Files          12      12              
  Lines         229     233       +4     
  Branches       21      24       +3     
=========================================
- Hits           87      86       -1     
- Misses        132     135       +3     
- Partials       10      12       +2
Impacted Files Coverage Δ
...erset-ui-plugin-chart-word-cloud/src/buildQuery.ts 100% <ø> (ø) ⬆️
...ugin-chart-word-cloud/src/legacy/transformProps.ts 54.54% <54.54%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db42df2...06b2e61. Read the comment docs.

export const ROTATION = {
flat: () => 0,
/* eslint-disable-next-line no-magic-numbers */
random: () => Math.floor(Math.random() * 6 - 3) * 30,
Copy link

@espressoroaster espressoroaster Nov 17, 2019

Choose a reason for hiding this comment

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

Maybe leave a comment to explain that this calculates a random rotation between -90 and 90 degrees. I'm assuming the unit is degrees since square is * 90, but it would be helpful to read it explicitly when skimming the code.

Copy link

@espressoroaster espressoroaster left a comment

Choose a reason for hiding this comment

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

I've only taken a look at Encoder.ts and Wordcloud.tsx, but those two LGTM. Have a non-blocking comment about adding a comment for the rotation function.

@kristw kristw changed the title feat: convert word cloud to use encodable feat(plugin-chart-word-cloud): convert word cloud to use encodable Nov 18, 2019
@kristw kristw merged commit 9b14285 into master Nov 20, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--word-cloud-encodable branch November 20, 2019 06:10
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
…pache-superset#258)

* feat: convert word cloud to use encodable

* fix: minor bugs

* feat: bump dependencies

* feat: use field

* fix: defaultProps

* fix: unit test

* docs: update storybook

* refactor: move files

* docs: update storybook

* fix: unit test

* fix: address comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants