Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

refactor(chart): remove and rename fields in ChartProps #174

Merged
merged 4 commits into from
Aug 15, 2019

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Jun 13, 2019

💔 Breaking Changes

BREAKING CHANGE: ChartProps fields are removed and renamed.

The functions (onAddFilter, onError, setControlValue, setTooltip) are nested under hooks because these are used primarily in Superset app and has no use for other apps.

Before After Note
annotationData annotationData
datasource datasource
filters *initialValues filters was misleading because it is actually initial values of the filter_box and table vis.
formData formData
hooks hooks
onAddFilter *hooks.onAddFilter No longer provide an empty function as default value
onError *hooks.onError No longer provide an empty function as default value
payload *queryData payload is ambiguous
setControlValue *hooks.setControlValue No longer provide an empty function as default value
setTooltip *hooks.setTooltip No longer provide an empty function as default value
width width
height height

Only merge this when we are ready to publish it along with other breaking changes.

@kristw kristw requested a review from a team as a code owner June 13, 2019 01:48
@netlify
Copy link

netlify bot commented Jun 13, 2019

Deploy preview for superset-ui ready!

Built with commit 3dfe5dd

https://deploy-preview-174--superset-ui.netlify.com

@kristw kristw changed the title [WIP] refactor(chart): remove and remove fields in ChartProps [WIP] refactor(chart): remove and rename fields in ChartProps Jun 18, 2019
@kristw kristw force-pushed the kristw--chartp branch 2 times, most recently from 2656f4d to a587d63 Compare August 13, 2019 19:51
@kristw kristw changed the title [WIP] refactor(chart): remove and rename fields in ChartProps refactor(chart): remove and rename fields in ChartProps Aug 13, 2019
@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

Merging #174 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files         102      107       +5     
  Lines        1222     1240      +18     
  Branches      293      305      +12     
==========================================
+ Hits         1221     1239      +18     
  Partials        1        1
Impacted Files Coverage Δ
...ackages/superset-ui-chart/src/models/ChartProps.ts 100% <100%> (ø) ⬆️
...ckages/superset-ui-chart/src/models/ChartPlugin.ts 100% <0%> (ø) ⬆️
...ges/superset-ui-encodeable/src/utils/isDisabled.ts 100% <0%> (ø)
...uperset-ui-encodeable/src/typeGuards/ChannelDef.ts 100% <0%> (ø)
...kages/superset-ui-encodeable/src/utils/identity.ts 100% <0%> (ø)
...ages/superset-ui-encodeable/src/typeGuards/Base.ts 100% <0%> (ø)
...ages/superset-ui-encodeable/src/utils/isEnabled.ts 100% <0%> (ø)

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 920d389...3dfe5dd. Read the comment docs.

@kristw kristw added the reviewable Ready for review label Aug 14, 2019
@kristw kristw changed the title refactor(chart): remove and rename fields in ChartProps refactor(chart): remove and rename props of SuperChart Aug 14, 2019
@kristw kristw changed the title refactor(chart): remove and rename props of SuperChart refactor(chart): remove and rename fields in ChartProps Aug 14, 2019
Copy link
Contributor

@etr2460 etr2460 left a comment

Choose a reason for hiding this comment

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

lgtm

@kristw kristw merged commit b53b839 into master Aug 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--chartp branch August 15, 2019 21:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewable Ready for review size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants