Skip to content

fix(tooltip): placement with left/top legends and single bars#771

Merged
nickofthyme merged 9 commits intoelastic:masterfrom
nickofthyme:fix-tooltip-placement
Aug 3, 2020
Merged

fix(tooltip): placement with left/top legends and single bars#771
nickofthyme merged 9 commits intoelastic:masterfrom
nickofthyme:fix-tooltip-placement

Conversation

@nickofthyme
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme commented Jul 30, 2020 •
edited
Loading

Summary

Fixes #769 and #770

Related to #259

Left and Top Legend (#769)

Before

Screen Recording 2020-07-30 at 11 35 AM

After

Screen Recording 2020-07-30 at 11 25 AM

Single value bars (#770)

Reverses changes from #259 in favor of new popperjs changes that allow greater tooltip customization.

Before

Screen Recording 2020-07-30 at 12 03 PM

After

Screen Recording 2020-07-30 at 12 05 PM

Note: The ordinal tooltip uses a top placement because there is no room on left or right. This can be fixed by setting the tooltip.boundary prop. However, storybook renders the chart in an iframe that prevents us from fixing this.

Checklist

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • This was checked for cross-browser compatibility, including a check against IE11
  • Proper documentation or storybook story was added for features that require explanation or tutorials
  • Unit tests were updated or added to match the most common scenarios

@nickofthyme nickofthyme added bug Something isn't working :tooltip Related to hover tooltip :xy Bar/Line/Area chart related labels Jul 30, 2020
@nickofthyme nickofthyme requested a review from markov00 July 30, 2020 16:40
@nickofthyme nickofthyme linked an issue Jul 30, 2020 that may be closed by this pull request
Bad tooltip placement for single bars #770
Closed
@nickofthyme nickofthyme changed the title fix(tooltip): placement with left and top legends fix(tooltip): placement with left and top legends and single bars Jul 30, 2020
@nickofthyme nickofthyme marked this pull request as ready for review July 30, 2020 18:07
@nickofthyme nickofthyme changed the title fix(tooltip): placement with left and top legends and single bars fix(tooltip): placement with left/top legends and single bars Jul 30, 2020
@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Jul 31, 2020

Hey @nickofthyme why the tooltip behavior of a single bar histogram is different than the stacked bar one?
In this gif I've added multiple histograms stacked on each other and seems that this behavior is actually fine and better than pushing the tooltip outside the chart

Jul-31-2020 10-28-55

@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Jul 31, 2020

We are going to integrate soon the small multiples, which means that the tooltip has to work over different areas independently from the legend position or its own panel. Is there a way to fix this without coupling this with the legend size? can we constraint the tooltip anchor to the chart panel only? Looking at the Before screenshots seems that the issue is related on the fact that we are not limiting the anchor inside the chart panel, do you think it's possible to achieve that without knowing the legend position/size?

markov00
markov00 approved these changes Aug 3, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Thanks for the legend size decouping, I've just one minor thing to ask about renaming:
can the globalChartDimension be renamed to globalOffset or only offset?
For everything else is good to merge after fixing the snapshot test

@nickofthyme nickofthyme merged commit e576b26 into elastic:master Aug 3, 2020
@nickofthyme nickofthyme deleted the fix-tooltip-placement branch August 3, 2020 15:25
markov00 pushed a commit that referenced this pull request Aug 10, 2020
@semantic-release-bot
chore(release): 21.0.0 [skip ci]
e7a8a30 
# [21.0.0](v20.0.2...v21.0.0) (2020-08-10)

### Bug Fixes

* update dep vulnerabilities, minimist and kind-of ([#763](#763)) ([4455281](4455281))
* **legend:** fix color anchor, add action context, fix action padding ([#774](#774)) ([4590a22](4590a22))
* **tooltip:** placement with left/top legends and single bars ([#771](#771)) ([e576b26](e576b26)), closes [#769](#769) [#770](#770)

### Features

* streamgraph and fit functions on stacked charts ([#751](#751)) ([268fcc0](268fcc0)), closes [#766](#766) [#715](#715) [#450](#450)

### BREAKING CHANGES

* the first parameter of `PointStyleAccessor` and `BarStyleAccessor` callbacks is changed from `RawDataSeriesDatum` to `DataSeriesDatum`. `stackAsPercentage` prop is replaced by `stackMode` that accept one `StackMode`.
@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Aug 10, 2020

🎉 This PR is included in version 21.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Aug 10, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@semantic-release-bot
chore(release): 21.0.0 [skip ci]
5c68eec 
# [21.0.0](elastic/elastic-charts@v20.0.2...v21.0.0) (2020-08-10)

### Bug Fixes

* update dep vulnerabilities, minimist and kind-of ([opensearch-project#763](elastic/elastic-charts#763)) ([843554f](elastic/elastic-charts@843554f))
* **legend:** fix color anchor, add action context, fix action padding ([opensearch-project#774](elastic/elastic-charts#774)) ([262f8d2](elastic/elastic-charts@262f8d2))
* **tooltip:** placement with left/top legends and single bars ([opensearch-project#771](elastic/elastic-charts#771)) ([75533b1](elastic/elastic-charts@75533b1)), closes [opensearch-project#769](elastic/elastic-charts#769) [opensearch-project#770](elastic/elastic-charts#770)

### Features

* streamgraph and fit functions on stacked charts ([opensearch-project#751](elastic/elastic-charts#751)) ([6f6a8cb](elastic/elastic-charts@6f6a8cb)), closes [opensearch-project#766](elastic/elastic-charts#766) [opensearch-project#715](elastic/elastic-charts#715) [opensearch-project#450](elastic/elastic-charts#450)

### BREAKING CHANGES

* the first parameter of `PointStyleAccessor` and `BarStyleAccessor` callbacks is changed from `RawDataSeriesDatum` to `DataSeriesDatum`. `stackAsPercentage` prop is replaced by `stackMode` that accept one `StackMode`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markov00 markov00 markov00 approved these changes

Assignees

No one assigned

Labels

bug Something isn't working released Issue released publicly :tooltip Related to hover tooltip :xy Bar/Line/Area chart related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bad tooltip placement for single bars Bad tooltip placement with left/top legend

2 participants

@nickofthyme @markov00