feat: expose Specs types and clean up annotation types#554
Merged
markov00 merged 4 commits intoelastic:masterfrom Feb 28, 2020
Merged
feat: expose Specs types and clean up annotation types#554markov00 merged 4 commits intoelastic:masterfrom
markov00 merged 4 commits intoelastic:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #554 +/- ##
=========================================
+ Coverage 71.68% 71.98% +0.3%
=========================================
Files 200 212 +12
Lines 6032 6186 +154
Branches 1167 1185 +18
=========================================
+ Hits 4324 4453 +129
- Misses 1690 1714 +24
- Partials 18 19 +1
Continue to review full report at Codecov.
|
2c0d538 to
fee5e2c
Compare
nickofthyme
approved these changes
Feb 27, 2020
Collaborator
nickofthyme
left a comment
There was a problem hiding this comment.
LGTM, Does this solve the issue Wylie mentioned about the spec types?
src/index.ts
Outdated
| export { ChartSize, ChartSizeArray, ChartSizeObject } from './utils/chart_size'; | ||
|
|
||
| // DEPRECATED: we will remove these types soon | ||
| export { SpecId, GroupId, AxisId, AnnotationId, getAxisId, getGroupId, getSpecId, getAnnotationId } from './utils/ids'; |
Collaborator
There was a problem hiding this comment.
I think you mean remove from export but I saw something in kibana that I started doing in the color picker PR which is for mapped types, and others that make sense, using SeriesId rather than string. It makes no difference to the output or consumer facing types, so we wouldn't need to export them. Something like...
// Good
type SeriesCollection = Map<SeriesId, SeriesIdentifier>;
// Bad
type SeriesCollection = Map<string, SeriesIdentifier>;Any thoughts on this?
Collaborator
Author
There was a problem hiding this comment.
You are right, let's keep these
| export { DARK_THEME } from './utils/themes/dark_theme'; | ||
|
|
||
| // utilities | ||
| export { RecursivePartial } from './utils/commons'; |
Collaborator
There was a problem hiding this comment.
maybe we could have a utils/index for all the utils we want to export.
Collaborator
Author
There was a problem hiding this comment.
I will address this in a different PR. Currently the Utils directory has more than utilities (also some types, the theme....it's a little mess there, I will try to move things around)
This commit removes the getSpecId, getAnnotationId, getGroupId, getAxisId getters in favour of directly using a string field BREAKING CHANGE: The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no more available, you can use directly a `string` for those ids
nickofthyme
approved these changes
Feb 28, 2020
Collaborator
nickofthyme
left a comment
There was a problem hiding this comment.
Crazy this is finally officially a thing of the past.
markov00
added a commit
that referenced
this pull request
Mar 2, 2020
This commit fix some wrongly configured annotation TS types and expose the Specs types as requested by #547. It also expose some more types that can be used by consumers that where buried inside our chart library. Finally it removes the `getSpecId`, `getAnnotationId`, `getGroupId`, `getAxisId` getters in favour of just using a string. BREAKING CHANGE: The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead. close #547
markov00
pushed a commit
that referenced
this pull request
Mar 17, 2020
# [18.0.0](v17.1.1...v18.0.0) (2020-03-17) ### Code Refactoring * clean up TS types ([#554](#554)) ([22f7635](22f7635)), closes [#547](#547) [#547](#547) * decouple tooltip from XY chart ([#553](#553)) ([e70792e](e70792e)), closes [#246](#246) ### Features * cleaner color API on SeriesSpec ([#571](#571)) ([f769f7c](f769f7c)) * **legend:** allow color picker component render prop ([#545](#545)) ([90f4b95](90f4b95)) * **partition:** add element click, over and out events ([#578](#578)) ([103df02](103df02)) * **partition:** add tooltip ([#544](#544)) ([6bf9a69](6bf9a69)), closes [#246](#246) * percentage display in partitioning charts ([#558](#558)) ([d6aa8d7](d6aa8d7)) * specify series name with a function on SeriesSpec ([#539](#539)) ([358455a](358455a)) * xAccessor can be a function accessor ([#574](#574)) ([bcc3d63](bcc3d63)) ### BREAKING CHANGES * The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead. * `customSeriesColors` prop on `SeriesSpec` is now `color`. The `CustomSeriesColors` type is replaced with `SeriesColorAccessor`. * Remove `customSubSeriesName` prop on series specs in favor of cleaner api using just the `name` prop on `SeriesSpec`. The types `SeriesStringPredicate`, `SubSeriesStringPredicate` have been removed. * the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.
2 tasks
AMoo-Miki
pushed a commit
to AMoo-Miki/OpenSearch-Dashboards
that referenced
this pull request
Feb 10, 2022
# [18.0.0](elastic/elastic-charts@v17.1.1...v18.0.0) (2020-03-17) ### Code Refactoring * clean up TS types ([opensearch-project#554](elastic/elastic-charts#554)) ([6ce56fa](elastic/elastic-charts@6ce56fa)), closes [opensearch-project#547](elastic/elastic-charts#547) [opensearch-project#547](elastic/elastic-charts#547) * decouple tooltip from XY chart ([opensearch-project#553](elastic/elastic-charts#553)) ([cb02cd0](elastic/elastic-charts@cb02cd0)), closes [opensearch-project#246](elastic/elastic-charts#246) ### Features * cleaner color API on SeriesSpec ([opensearch-project#571](elastic/elastic-charts#571)) ([6a78c4e](elastic/elastic-charts@6a78c4e)) * **legend:** allow color picker component render prop ([opensearch-project#545](elastic/elastic-charts#545)) ([22ef1e6](elastic/elastic-charts@22ef1e6)) * **partition:** add element click, over and out events ([opensearch-project#578](elastic/elastic-charts#578)) ([4189573](elastic/elastic-charts@4189573)) * **partition:** add tooltip ([opensearch-project#544](elastic/elastic-charts#544)) ([0cffed4](elastic/elastic-charts@0cffed4)), closes [opensearch-project#246](elastic/elastic-charts#246) * percentage display in partitioning charts ([opensearch-project#558](elastic/elastic-charts#558)) ([993a448](elastic/elastic-charts@993a448)) * specify series name with a function on SeriesSpec ([opensearch-project#539](elastic/elastic-charts#539)) ([fc6430b](elastic/elastic-charts@fc6430b)) * xAccessor can be a function accessor ([opensearch-project#574](elastic/elastic-charts#574)) ([f702e2c](elastic/elastic-charts@f702e2c)) ### BREAKING CHANGES * The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead. * `customSeriesColors` prop on `SeriesSpec` is now `color`. The `CustomSeriesColors` type is replaced with `SeriesColorAccessor`. * Remove `customSubSeriesName` prop on series specs in favor of cleaner api using just the `name` prop on `SeriesSpec`. The types `SeriesStringPredicate`, `SubSeriesStringPredicate` have been removed. * the `SeriesIdentifier` type is generalized into a simplified object with two values in common: `specId` and `key`. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier`. The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` and its default value is now `false` hiding the current/last value on the legend by default.
nickofthyme
added a commit
to nickofthyme/kibana
that referenced
this pull request
Mar 28, 2024
The chart ids used to be cast as unique strings (https://github.com/markov00/elastic-charts/blame/8afa62afc4206d8ce3876aa5ccd9c69a8c20518a/src/utils/ids.ts) but this was changed in elastic/elastic-charts#281 and fully removed in elastic/elastic-charts#554.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fix some wrongly configured annotation TS types and expose the Specs types as requested by #547
It also expose some more types that can be used by consumers that where buried inside our chart library.
This commit removes also the
getSpecId,getAnnotationId,getGroupId,getAxisIdgetters in favour of just using a stringBREAKING CHANGE: The
getSpecId,getGroupId,getAxisIdandgetAnnotationIdare no more available, you can use directly astringfor those ids.close #547
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.src/index.ts(and stories only import from../srcexcept for test data & storybook)