-
Notifications
You must be signed in to change notification settings - Fork 121
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
feat(wordcloud): wordcloud #1038
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💚 CLA has been signed |
Hi @Kati-dev-hu thanks so much for that, we really appreciate it!
Anyway I think this already works very well and looks very clean, thanks for this big help! |
Hi @markov00, thanks for your comments. Text weight is resolved, and the logic for missing words is coming up too, pushing commits soon |
61d147b
to
a1e9f2c
Compare
a1e9f2c
to
0cda284
Compare
# Conflicts: # .playground/playground.tsx # api/charts.api.md
Codecov Report
@@ Coverage Diff @@
## master #1038 +/- ##
==========================================
- Coverage 72.72% 72.20% -0.52%
==========================================
Files 367 397 +30
Lines 11412 12058 +646
Branches 2479 2560 +81
==========================================
+ Hits 8299 8707 +408
- Misses 3098 3328 +230
- Partials 15 23 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
jenkins test this please |
jenkins test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks great, I've added few comments that we can easily fix on a subsequent PR.
Thank you very much for this great contribution!
}); | ||
|
||
/** @public */ | ||
export type WeightFun = Values<typeof WeightFun>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Function
is usually abbreviated as Fn
outOfRoomCallback: (wordCount, renderedWordCount) => { | ||
Logger.warn(`Not all words have been placed: ${renderedWordCount} words rendered out of ${wordCount}`); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure about having that console log by default. We can probably have a no-op here
private tryCanvasContext() { | ||
const canvas = this.props.forwardStageRef.current; | ||
this.ctx = canvas && canvas.getContext('2d'); | ||
} | ||
|
||
private drawCanvas() { | ||
if (this.ctx) { | ||
/* const { width, height }: Dimensions = this.props.chartContainerDimensions; | ||
renderCanvas2d(this.ctx, this.devicePixelRatio, { | ||
...this.props.geometries, | ||
config: { ...this.props.geometries.config, width, height }, | ||
}); | ||
*/ | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cam we completely remove the canvas element here and on the rendering? If we are not using it right now it's better to clean this up
} | ||
|
||
getTooltipInfo(globalState: GlobalChartState) { | ||
return getTooltipInfoSelector(globalState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tooltip doesn't bring any new info as it is proposed today. We can easily disable it
# [25.4.0](v25.3.0...v25.4.0) (2021-03-23) ### Bug Fixes * chromium area path render bug ([#1067](#1067)) ([e16d15d](e16d15d)) ### Features * **tooltip:** expose datum in the TooltipValue ([#1082](#1082)) ([0246784](0246784)), closes [#1042](#1042) * **wordcloud:** wordcloud ([#1038](#1038)) ([f08f4c9](f08f4c9))
🎉 This PR is included in version 25.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [25.4.0](elastic/elastic-charts@v25.3.0...v25.4.0) (2021-03-23) ### Bug Fixes * chromium area path render bug ([opensearch-project#1067](elastic/elastic-charts#1067)) ([37301bf](elastic/elastic-charts@37301bf)) ### Features * **tooltip:** expose datum in the TooltipValue ([opensearch-project#1082](elastic/elastic-charts#1082)) ([48dc9d5](elastic/elastic-charts@48dc9d5)), closes [opensearch-project#1042](elastic/elastic-charts#1042) * **wordcloud:** wordcloud ([opensearch-project#1038](elastic/elastic-charts#1038)) ([d724cad](elastic/elastic-charts@d724cad))
Summary
Initial wordcloud implementation
Checklist
Delete any items that are not applicable to this PR.
src/index.ts
(and stories only import from../src
except for test data & storybook)