feat(interaction): crosshair #96

markov00 · 2019-03-08T22:47:03Z

close #80
close #58
close #88

Summary

This PR introduce the Crosshair interaction.

We have removed every listeners to canvas elements. We are instead using a map for each value on the chart: this map maps x value and any relative Y values associated. We then just invert the x scale relative to the mouse point to get the right X key to use on that map. This increase the performance on highly/medium dense charts.

We added two new props to Settings:

tooltipType: that describe the type of tooltip/crosshair to use Crosshair,VerticalCursor,Follow, None
tooltipSnap: that define if we want to tooltip to snap to the grid/x axis values or not for line/area charts

NOTE
The inverted function on time scales, currently works correctly when using UTC dates. Further investigation needs to be done to make that works with any time zone. To test this check Interactions - brush selection tool on time charts story and remove the Z from the now constant time.

ToDo:

Checklist

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

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
Each commit follows the convention

codecov-io · 2019-03-18T18:32:48Z

Codecov Report

Merging #96 into master will increase coverage by 0.54%.
The diff coverage is 88.6%.

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   89.63%   90.17%   +0.54%     
==========================================
  Files          28       31       +3     
  Lines        1167     1405     +238     
  Branches      119      150      +31     
==========================================
+ Hits         1046     1267     +221     
- Misses        111      125      +14     
- Partials       10       13       +3

Impacted Files	Coverage Δ
src/lib/themes/light_theme.ts	`100% <ø> (ø)`	⬆️
src/lib/utils/dimensions.ts	`76.92% <ø> (ø)`	⬆️
src/lib/themes/dark_theme.ts	`100% <ø> (ø)`	⬆️
src/lib/series/specs.ts	`100% <ø> (ø)`	⬆️
src/lib/series/domains/y_domain.ts	`100% <ø> (+1.58%)`	⬆️
src/lib/series/legend.ts	`100% <100%> (ø)`	⬆️
src/lib/series/domains/x_domain.ts	`100% <100%> (ø)`	⬆️
src/lib/series/series.ts	`92.96% <100%> (ø)`	⬆️
src/lib/themes/theme.ts	`100% <100%> (ø)`	⬆️
src/lib/utils/scales/scales.ts	`100% <100%> (ø)`	⬆️
... and 19 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 43578e0...f75f77f. Read the comment docs.

emmacunningham

tested locally on Firefox & looks good! Just left a few comments & questions here

src/components/crosshair.tsx

emmacunningham · 2019-03-19T05:57:17Z

src/components/react_canvas/area_geometries.tsx

            strokeHitEnabled={false}
            listening={false}
-            perfectDrawEnabled={true}
+            perfectDrawEnabled={false}


To maintain consistency of the props between the animated and non-animated versions, could create an object with the shared props and spread them? (Here and for other animated components)

will work on that on a different PR

src/components/tooltips.tsx

src/lib/series/series_utils.ts

src/state/chart_state.ts

emmacunningham · 2019-03-19T14:33:14Z

src/state/chart_state.ts

-    if (this.onElementOverListener) {
-      this.onElementOverListener(tooltip.value);
+
+  setCursorPosition = action((x: number, y: number) => {


Perhaps some of the computation in this function can be pulled into utils as pure functions which (hopefully) will make test writing a bit easier.

partially solved 7212d51

src/state/chart_state.ts

src/state/crosshair_utils.ts

emmacunningham

Code changes LGTM 👍 and tested locally on Firefox

fixed crosshair/tooltip positioning on rotated charts, moved computation to state, add testing for set cursor function

Refactor based on review. Removed an unused function

for ordinal chart with line/area only, we have to take in consideration that the bandwidth of the cursorBand is the same as the bandwidth of the scale, and we don't have to take in consideration that the totalgroupcount is 0 (that compute only bar groups)

I've refactored the name because it's a bit more clear to read and understand than a generic groupCount. In fact, that value counts the number of bars clusteres on each x value.

Refactored the mergeYdomains to avoid throwing that will never happens

markov00 · 2019-03-22T16:34:40Z

🎉 This PR is included in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

markov00 added wip work in progress :interactions Interactions related issue labels Mar 8, 2019

This was referenced Mar 8, 2019

Roadmap #3

Closed

Preparation for TSVB timeseries chart replacement #64

Closed

markov00 force-pushed the feature/crosshair branch 2 times, most recently from a848e45 to d54a2cb Compare March 18, 2019 18:22

markov00 force-pushed the feature/crosshair branch from d54a2cb to 76f6b48 Compare March 18, 2019 21:50

emmacunningham reviewed Mar 19, 2019

View reviewed changes

emmacunningham approved these changes Mar 21, 2019

View reviewed changes

markov00 added 13 commits March 22, 2019 15:44

feat: add crosshair

b3a8ebd

refactor: fix highlight typos

ad8fab5

refactor: fix missing typos on chart_state

98af34f

style: remove unused commented code

a0033c9

refactor(crosshair): cleaned up crosshair and tooltip components

2079c1c

fixed crosshair/tooltip positioning on rotated charts, moved computation to state, add testing for set cursor function

refactor: fix typos, defaultMinInterval

b399e76

Refactor based on review. Removed an unused function

test: increase coverage for interactions

309746a

refactor: renamed totalGroupCount to totalBarsInCluster

db5d5b6

I've refactored the name because it's a bit more clear to read and understand than a generic groupCount. In fact, that value counts the number of bars clusteres on each x value.

test(rendering): add rendering extensive tests for lines/area/bars

c161b51

test: increase test coverage for rendering

b47077b

test: increase coverage for theme

d59a4a4

test: increase the coverage for domains

82a18d3

Refactored the mergeYdomains to avoid throwing that will never happens

markov00 force-pushed the feature/crosshair branch from 5246166 to 82a18d3 Compare March 22, 2019 14:47

markov00 added 2 commits March 22, 2019 15:57

test: fix missing merged arguments

01c93fc

test: increase test coverage on chart state

f75f77f

markov00 removed the wip work in progress label Mar 22, 2019

markov00 merged commit 5ddd1a8 into elastic:master Mar 22, 2019

markov00 added the released Issue released publicly label Mar 22, 2019

markov00 added this to the 7.1 milestone Mar 26, 2019

markov00 mentioned this pull request Mar 29, 2019

Test tooltip on high density chart #19

Closed

markov00 removed this from the 7.1 milestone Apr 1, 2019

markov00 deleted the feature/crosshair branch April 16, 2019 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(interaction): crosshair #96

feat(interaction): crosshair #96

markov00 commented Mar 8, 2019 •

edited

Loading

codecov-io commented Mar 18, 2019 •

edited

Loading

emmacunningham left a comment

emmacunningham Mar 19, 2019

markov00 Mar 21, 2019

emmacunningham Mar 19, 2019

markov00 Mar 21, 2019

emmacunningham left a comment

markov00 commented Mar 22, 2019

feat(interaction): crosshair #96

feat(interaction): crosshair #96

Conversation

markov00 commented Mar 8, 2019 • edited Loading

Summary

Checklist

codecov-io commented Mar 18, 2019 • edited Loading

Codecov Report

emmacunningham left a comment

Choose a reason for hiding this comment

emmacunningham Mar 19, 2019

Choose a reason for hiding this comment

markov00 Mar 21, 2019

Choose a reason for hiding this comment

emmacunningham Mar 19, 2019

Choose a reason for hiding this comment

markov00 Mar 21, 2019

Choose a reason for hiding this comment

emmacunningham left a comment

Choose a reason for hiding this comment

markov00 commented Mar 22, 2019

markov00 commented Mar 8, 2019 •

edited

Loading

codecov-io commented Mar 18, 2019 •

edited

Loading