-
Notifications
You must be signed in to change notification settings - Fork 190
fix: fix tooltip position with zoom bar enabled #732
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: fix tooltip position with zoom bar enabled #732
Conversation
0201d27
to
9d6dde0
Compare
} else { | ||
// if the mouse position is from event (ruler) | ||
// we need add zoom bar height | ||
if (isZoomBarEnabled) { |
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.
what if zoom bar gets positioned in the bottom of the chart?
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.
what if zoom bar gets positioned in the bottom of the chart?
The isZoomBarEnabled
is actually checking the top zoom bar.
So it won't change y position if zoom bar is in the bottom of the chart.
The variable name is updated to make it clear.
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.
Looks good overall 👍 Agree with @theiliad, it should account for top/bottom positioning
* fix: fix tooltip position with zoom bar enabled (carbon-design-system#732) * fix: fix tooltip position with zoom bar enabled * fix: change variable name to make it clear * Merge pull request carbon-design-system#737 from hlyang397/fix-join-problem * v0.34.9 * Merge pull request carbon-design-system#739 from theiliad/add-zoombar-slider * add separate slider view height values * feat(core, angular, vue, svelte, react): add slider view to zoombar * refactor * format * v0.34.10 * Merge pull request carbon-design-system#728 from sophiiae/legend-order * feat: add legend order option * refactor: sort legend only * fix: fix legend order default value * fix: solve partial defined ordering case * refactor: put legend ordering in separate function * fix: fix demo * fix: fix demo Co-authored-by: Fei <[email protected]> * v0.34.11 * Update README.md * feat(svelte): add Meter chart wrapper (carbon-design-system#744) * Merge pull request carbon-design-system#745 from hlyang397/support_mobile_event * v0.35.0 * Add files via upload * chore(monorepo): update logo (carbon-design-system#746) * Add files via upload * Update README.md * Merge pull request carbon-design-system#749 from theiliad/seperate-non-customizables * fix(core): separate out non-customizables in configuration.ts * format * v0.35.1 * Merge pull request carbon-design-system#756 from sophiiae/pie-donut-skeleton * fix(core, angular, vue, react, svelt): fix missing donut / pie skeleton * fix(core, angular, vue, react, svelt): fix pie/donut skeleton alignment * style(core, angular, vue, react, svelt): remove redundant variable Co-authored-by: Fei <[email protected]> * v0.35.2 * Merge pull request carbon-design-system#757 from natashadecoste/react-events-fix * chore(angular): add angular 9 dependency versions to peerDependencies * fix(react): compare props before updating component to avoid re-render of same chart * compare props with new props for all charts Co-authored-by: Eliad Moosavi <[email protected]> * v0.35.3 * feat: enable chart grid enable option * refactor: rename grid option * feat: change zoom bar handle cursor to ew-resize (carbon-design-system#759) * feat: change zoom bar handle cursor to ew-resize * Update zoom-bar.ts Co-authored-by: Eliad Moosavi <[email protected]> * Merge pull request carbon-design-system#760 from hlyang397/avoid-duplicate-spacer * v0.36.0 * refactor: rename gridEnabled to isGridEnabled * fix: enable grid as default setting * feat: enable or disable x and y grid separately * feat: enable or disable ruler * feat: enable or disable tooltip * fix: enable tooltip as default setting * fix: enable ruler as default setting * fix: not positioning tooltip when tooltip is disabled * feat: enable or disable scatter dot on charts except scatter chart * refactor: only go through init and render when isTooltipEnabled is true * refactor: move isRulerEnabled into render * refactor: remove backdrop event listener * refactor: rename variable * feat: gradient util * feat: refactor and add sparkline storybook in area chart section * refactor: eslint error * fix: set default gradientEnabled to false * refactor: remove class variable isTooltipEnabled * refactor: interface modification for xGridEnabled and yGridEnabled * feat: hide or show axis * Merge pull request carbon-design-system#779 from scottdickerson/fix-tooltip-valueformatter * v0.36.1 * Merge pull request carbon-design-system#764 from hlyang397/fix-scatter-stacked-tooltip * fix: stacked scatter tooltip not show - abstract getTooltipData function to allow override - implement getTooltipData function in scatter-stacked with stacked data * refactor: change variable name from data to datum * refactor: updatable tooltip event listener * refactor: rename grid option * v0.36.2 * fix: allow apply gradient style when there is only one dataset * refactor: only render needed code when axis is disabled * refactor: flag for checking whether tooltip event listener is added or not * feat: sparkline loading state * feat: sparkline loading storybook * refactor: rename class * refactor: remove code for styling backdrop in ruler component * Merge branch 'sparkline-loading' of https://github.com/JennChao/carbon-charts into sparkline-for-demo * fix: only apply custom options to sparkline chart Co-authored-by: Eric Yang <[email protected]> Co-authored-by: carbon-bot <[email protected]> Co-authored-by: Eliad Moosavi <[email protected]> Co-authored-by: Fei Z <[email protected]> Co-authored-by: Fei <[email protected]> Co-authored-by: Eric Liu <[email protected]> Co-authored-by: natashadecoste <[email protected]> Co-authored-by: Eliad Moosavi <[email protected]> Co-authored-by: Jennifer Chao <[email protected]> Co-authored-by: Scott Dickerson <[email protected]>
…#732) * fix: fix tooltip position with zoom bar enabled * fix: change variable name to make it clear
Updates
Demo screenshot or recording
before:

after:

Review checklist (for reviewers only)