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

feat: improve line chart margin/axis and add buildquery #66

Merged
merged 7 commits into from
Apr 18, 2019

Conversation

kristw
Copy link
Collaborator

@kristw kristw commented Apr 18, 2019

🏆 Enhancements

  • feat: can disable axis title
  • feat: adjust margin according to axis title visibility
  • feat: add type for line chart's formdata
  • feat: add buildQuery for line chart

📜 Documentation

  • docs: add more knobs to storybook

🐛 Bug Fix

  • fix: fallback to default margin when margin is partially set

🏠 Internal

  • chore: bump storybook addons version

@kristw kristw requested a review from a team as a code owner April 18, 2019 00:00
Copy link

@schillerk schillerk left a comment

Choose a reason for hiding this comment

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

Woo thanks!

const defaultProps = {
className: '',
margin: { top: 20, right: 20, left: 20, bottom: 20 },
margin: DEFAULT_MARGIN,

Choose a reason for hiding this comment

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

I don't think you need this since you're doing margin: { ...DEFAULT_MARGIN, ...margin }, down below anyway

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is useful for deriving the type as well. (typeof defaultProps)

@@ -4,13 +4,14 @@ import { ChartProps } from '@superset-ui/chart';

export default function transformProps(chartProps: ChartProps) {
const { width, height, formData, payload } = chartProps;
const { encoding } = formData;
const { encoding, margin } = formData;
const { data } = payload;

return {
data,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, we need to implement get_data logic here. https://github.com/apache/incubator-superset/blob/master/superset/viz.py#L1222

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I don't implement anything, will it behave just like the word cloud query? I plan to get this pr in as a starter and will develop the buildQuery in a separate pr.

@netlify
Copy link

netlify bot commented Apr 18, 2019

Deploy preview for superset-ui-plugins ready!

Built with commit 799b3dd

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

@netlify
Copy link

netlify bot commented Apr 18, 2019

Deploy preview for superset-ui-plugins ready!

Built with commit 01481c1

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

Copy link
Contributor

@conglei conglei left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for adding this :)

@kristw kristw merged commit 9c9dd9d into master Apr 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the kristw--default-margin branch April 18, 2019 21:15
nytai pushed a commit to preset-io/superset-ui-plugins that referenced this pull request Apr 27, 2020
…set#66)

* fix: fallback to default margin when margin is partially set

* feat: can disable axis title

* feat: adjust margin according to axis title visibility

* feat: include margin in formData

* feat: add buildQuery

* fix: address kyle comments

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

Successfully merging this pull request may close these issues.

3 participants