Skip to content

fix: position tooltip within chart with single value xScale#259

Merged
emmacunningham merged 6 commits intoelastic:masterfrom
emmacunningham:fix/single-datum-tooltip
Jul 15, 2019
Merged

fix: position tooltip within chart with single value xScale#259
emmacunningham merged 6 commits intoelastic:masterfrom
emmacunningham:fix/single-datum-tooltip

Conversation

@emmacunningham
Copy link
Copy Markdown
Contributor

@emmacunningham emmacunningham commented Jul 3, 2019 •
edited
Loading

Summary

fix #254

This PR introduces a fix for the tooltip positioning on a chart with a single value xScale.

I'm not entirely sure where the best place to position the tooltip, so this presents the minimal effort version, where the tooltip will be positioned inside of the chart aligned along the chart's edge (left if horizontal rotation, top if vertical rotation):

single_value

Implementation-wise, we've added a method isSingleValue to the Scale classes so that we can check if a scale is a single value scale (so that we can apply the different positioning in that case). The customDomain knob was added to the stories to demonstrate that, when the chart only has one datapoint but has a multiple value scale, the tooltips are positioned as usual (since there is room to display them as normal).

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
- [ ] Proper documentation or storybook story was added for features that require explanation or tutorials

@emmacunningham emmacunningham added wip work in progress :interactions Interactions related issue labels Jul 3, 2019
@emmacunningham emmacunningham changed the title Fix/single datum tooltip fix: position tooltip within chart with single value xScale Jul 3, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 3, 2019 •
edited
Loading

Codecov Report

Merging #259 into master will increase coverage by 0.41%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #259      +/-   ##
==========================================
+ Coverage   97.84%   98.26%   +0.41%     
==========================================
  Files          36       36              
  Lines        2693     3170     +477     
  Branches      607      774     +167     
==========================================
+ Hits         2635     3115     +480     
+ Misses         51       44       -7     
- Partials        7       11       +4
Impacted Files Coverage Δ
src/lib/utils/scales/scale_band.ts 94.11% <100%> (+0.36%) ⬆️
src/state/crosshair_utils.ts 91.66% <100%> (+6.37%) ⬆️
src/lib/utils/interactions.ts 100% <100%> (ø) ⬆️
src/lib/utils/scales/scale_continuous.ts 94.11% <100%> (+0.31%) ⬆️
src/state/chart_state.ts 97.44% <100%> (+0.54%) ⬆️
src/lib/series/stacked_series_utils.ts 96.47% <0%> (-1.75%) ⬇️
src/lib/series/specs.ts 100% <0%> (ø) ⬆️
src/lib/series/series.ts 100% <0%> (ø) ⬆️
src/lib/series/domains/y_domain.ts 100% <0%> (ø) ⬆️
src/lib/axes/canvas_text_bbox_calculator.ts 100% <0%> (ø) ⬆️
... and 6 more

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 af92736...121a8fb. Read the comment docs.

@emmacunningham emmacunningham marked this pull request as ready for review July 8, 2019 17:03
@emmacunningham emmacunningham removed the wip work in progress label Jul 8, 2019
@emmacunningham emmacunningham mentioned this pull request Jul 8, 2019
Closed
32 tasks
@emmacunningham emmacunningham added the bug Something isn't working label Jul 8, 2019
@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Jul 9, 2019

hey @emmacunningham just a suggestion, what if in the "single value" mode we switch between the "crosshair/vertical" mode and we manually enable the "follow" mode?

nickofthyme
nickofthyme requested changes Jul 9, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM. Just two comments.

@emmacunningham
Copy link
Copy Markdown
Contributor Author

emmacunningham commented Jul 9, 2019

hey @emmacunningham just a suggestion, what if in the "single value" mode we switch between the "crosshair/vertical" mode and we manually enable the "follow" mode?

@markov00 yes, good idea. have implemented that logic here: 121a8fb

nickofthyme
nickofthyme approved these changes Jul 10, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Changes look good

@emmacunningham emmacunningham merged commit f458bc9 into elastic:master Jul 15, 2019
markov00 pushed a commit that referenced this pull request Jul 15, 2019
@semantic-release-bot
chore(release): 8.0.1 [skip ci]
a8f99b1 
## [8.0.1](v8.0.0...v8.0.1) (2019-07-15)

### Bug Fixes

* position tooltip within chart with single value xScale ([#259](#259)) ([f458bc9](f458bc9))
@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Jul 15, 2019

🎉 This PR is included in version 8.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Jul 15, 2019
@nickofthyme nickofthyme mentioned this pull request Jul 30, 2020
Merged
4 tasks
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
@semantic-release-bot
chore(release): 8.0.1 [skip ci]
70b36d5 
## [8.0.1](elastic/elastic-charts@v8.0.0...v8.0.1) (2019-07-15)

### Bug Fixes

* position tooltip within chart with single value xScale ([opensearch-project#259](elastic/elastic-charts#259)) ([6c6c2d7](elastic/elastic-charts@6c6c2d7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickofthyme nickofthyme nickofthyme approved these changes

@markov00 markov00 Awaiting requested review from markov00

Assignees

No one assigned

Labels

bug Something isn't working :interactions Interactions related issue released Issue released publicly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Single datum chart on element hover tooltip overflows the chart

4 participants

@emmacunningham @codecov-io @markov00 @nickofthyme