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

fix(BarSeries): ignore histogram mode in determining stacked series #2225

Merged
merged 8 commits into from
Nov 6, 2023

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Nov 1, 2023

Summary

This PR removes the forced stacking of bar charts with enableHistogramMode set to true.

Before

Screen Recording 2023-11-01 at 09 51 39 AM

Notice the domain fit appears to be disabled when enableHistogramMode is used, due to the forced stacking.

After

Screen Recording 2023-11-01 at 04 20 31 PM

Notice the domain is able to fit the data with enableHistogramMode enabled even for many bar series. Now only stacking the series disables the fit behavior as required by the current filling logic used to stacking bars.

Details

This was added in #751 to prevent using clustered bars in a histogram bar chart.

We also implemented this logic in #2226 to avoid the forced stacking of histogram bars when only one series was present. However, this implementation would still impact previous fitting behavior allow in kibana related to elastic/kibana#170316, specifically fitting multiple unstacked histogram bars to data domain.

Issues

fix #2223

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated

@nickofthyme nickofthyme added bug Something isn't working :data Data/series/scales related issue :xy Bar/Line/Area chart related labels Nov 1, 2023
@nickofthyme
Copy link
Collaborator Author

buildkite update screenshots

@nickofthyme nickofthyme merged commit 27b4281 into elastic:main Nov 6, 2023
13 checks passed
@nickofthyme nickofthyme deleted the no-forced-hist-stacking branch November 6, 2023 20:03
nickofthyme pushed a commit that referenced this pull request Nov 8, 2023
# [61.0.0](v60.0.0...v61.0.0) (2023-11-08)

### Bug Fixes

* `onRenderChange` callback trigger on resize ([#2228](#2228)) ([be30c1b](be30c1b))
* **axis:** always render `tickLine` unless `visible` is `false` ([#2194](#2194)) ([ec95d50](ec95d50))
* **BarSeries:** ignore histogram mode in determining stacked series ([#2225](#2225)) ([27b4281](27b4281))
* clamp brushing min of last bucket ([#2227](#2227)) ([155c22d](155c22d))
* **deps:** update dependency @elastic/eui to ^88.5.0 ([#2179](#2179)) ([2bb921e](2bb921e))
* **deps:** update dependency @elastic/eui to ^88.5.4 ([#2190](#2190)) ([05b33e5](05b33e5))
* **deps:** update dependency @elastic/eui to ^89.1.0 ([#2212](#2212)) ([a91f68d](a91f68d))
* **deps:** update dependency @elastic/eui to v89 ([#2193](#2193)) ([132327d](132327d))
* **deps:** update dependency @elastic/eui to v90 ([#2222](#2222)) ([10cd53b](10cd53b))

### chore

* reclaim charts theme ownership from eui ([#2175](#2175)) ([422c7d5](422c7d5))

### Features

* **metric:** allow alpha colors and improve contrast logic  ([#2184](#2184)) ([dd5732e](dd5732e))

### BREAKING CHANGES

* **BarSeries:** now ignores histogram mode in determining stacked series
* elastic charts theme renamed to `LEGACY_DARK_THEME` and `LEGACY_LIGHT_THEME` in favor of the main `DARK_THEME` and `LIGHT_THEME` which was merged with eui theme overrides. These new themes are now default.
* **axis:** Now respects `tickLine.padding` whenever `tickLine.visible` is `true`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :data Data/series/scales related issue :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not force stacking on single histogram series
1 participant