-
Notifications
You must be signed in to change notification settings - Fork 122
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: decuple brush cursor from chart rendering #331
Conversation
This commit remove the unnecessary style recalculation caused by chancing the cursor style on the document body from the chart_state. This commit also decouple the chart container from the chart renderer, removing multiple rendering caused by the mouse leaving the brush area
21d6583
to
83ae1f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nickofthyme I've updated the PR description fixing the |
Codecov Report
@@ Coverage Diff @@
## master #331 +/- ##
=========================================
Coverage ? 98.51%
=========================================
Files ? 38
Lines ? 2689
Branches ? 621
=========================================
Hits ? 2649
Misses ? 37
Partials ? 3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
# [10.0.0](v9.2.1...v10.0.0) (2019-08-21) ### Bug Fixes * **tooltip:** ie11 flex sizing ([#334](#334)) ([abaa472](abaa472)), closes [#332](#332) * decuple brush cursor from chart rendering ([#331](#331)) ([789f85a](789f85a)), closes [elastic/kibana#36517](elastic/kibana#36517) * remove clippings from chart geometries ([#320](#320)) ([ed6d0e5](ed6d0e5)), closes [#20](#20) ### Features * auto legend resize ([#316](#316)) ([659d27e](659d27e)), closes [#268](#268) * customize number of axis ticks ([#319](#319)) ([2b838d7](2b838d7)) * **theme:** base theme prop ([#333](#333)) ([a9ff5e1](a9ff5e1)), closes [#292](#292) ### BREAKING CHANGES * **theme:** remove `baseThemeType` prop on `Settings` component and `BaseThemeTypes` type. * `theme.legend.spacingBuffer` added to `Theme` type. Controls the width buffer between the legend label and value.
🎉 This PR is included in version 10.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [10.0.0](elastic/elastic-charts@v9.2.1...v10.0.0) (2019-08-21) ### Bug Fixes * **tooltip:** ie11 flex sizing ([opensearch-project#334](elastic/elastic-charts#334)) ([2683333](elastic/elastic-charts@2683333)), closes [opensearch-project#332](elastic/elastic-charts#332) * decuple brush cursor from chart rendering ([opensearch-project#331](elastic/elastic-charts#331)) ([b5b8dde](elastic/elastic-charts@b5b8dde)), closes [elastic/kibana#36517](elastic/kibana#36517) * remove clippings from chart geometries ([opensearch-project#320](elastic/elastic-charts#320)) ([497efa4](elastic/elastic-charts@497efa4)), closes [opensearch-project#20](elastic/elastic-charts#20) ### Features * auto legend resize ([opensearch-project#316](elastic/elastic-charts#316)) ([be4a53d](elastic/elastic-charts@be4a53d)), closes [opensearch-project#268](elastic/elastic-charts#268) * customize number of axis ticks ([opensearch-project#319](elastic/elastic-charts#319)) ([a7a4ecd](elastic/elastic-charts@a7a4ecd)) * **theme:** base theme prop ([opensearch-project#333](elastic/elastic-charts#333)) ([0b38770](elastic/elastic-charts@0b38770)), closes [opensearch-project#292](elastic/elastic-charts#292) ### BREAKING CHANGES * **theme:** remove `baseThemeType` prop on `Settings` component and `BaseThemeTypes` type. * `theme.legend.spacingBuffer` added to `Theme` type. Controls the width buffer between the legend label and value.
Summary
This PR remove the unnecessary style recalculation caused by chancing the cursor style on the document body from the
chart_state
. This was noticeable on this PR elastic/kibana#36517 (review) where the style reflow calculation tooks up to 75ms.This PR decouple the chart container from the chart renderer, allowing to set correctly the cursor changing the class withour re-rendering the chart component, as the actual implementation.
The cursor style now depends also on the status of the highlighted geometries. If one geometry is highlighted but no
onElementClick
oronElementOver
listener is available, the cursor will bedefault
if the brush is disabled, orcrosshair
if the brush is Enabled.The pointer is shown only if we have one between
onElementClick
oronElementOver
enabled and we are over a geometryno brush no click/over
only brush
brush and click
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] Any consumer-facing exports were added tosrc/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[ ] Unit tests were updated or added to match the most common scenarios