Skip to content
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

[charts] Fix default stackOffset for LineChart #11647

Merged
merged 3 commits into from
Jan 16, 2024

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Jan 10, 2024

Fix issue reported in https://discord.com/channels/1131323012554174485/1131329761952682114/1194552540294025256

I update don e of the demo to make sure it works properly when a value is 0.

Tltr: The default stacking strategy was "diverging" which staks all the positive values on top of each other and all the negative values below each over. For example values 1, -2, 3, -4 will render rectangles between:

  • [0, 1]
  • [-2, 0]
  • [1, 4] (stacked on top of value 1)
  • [-6, -2] (stacked on bellow value -2)

The edge case being 0 which is always ploted at [0, 0]

As you can see in the image is does not work with area charts, because when a series has a 0, it moves to 0 instead of stacking on top of others

image

@alexfauquette alexfauquette added the component: charts This is the name of the generic UI component, not the React module! label Jan 10, 2024
@mui-bot
Copy link

mui-bot commented Jan 10, 2024

Deploy preview: https://deploy-preview-11647--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against 9ebaf5e

@alexfauquette alexfauquette added the needs cherry-pick The PR should be cherry-picked to master after merge label Jan 16, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Nice improvement. 💯

[key in 'appearance' | 'ascending' | 'descending' | 'insideOut' | 'none' | 'reverse']: (
series: Series<any, any>,
) => number[];
[key in StackOrderType]: (series: Series<any, any>) => number[];
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, this refactor makes code way cleaner. 👏 💯

@LukasTy LukasTy changed the title [charts] Fix default stackOffset for line chart [charts] Fix default stackOffset for LineChart Jan 16, 2024
@alexfauquette alexfauquette merged commit 444269e into mui:next Jan 16, 2024
18 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants