Do not show tooltip when we are blurring the chart #637
Conversation
🔍 Visual review for your branch is published 🔍Here are the links to: |
| {!yAxis?.isBlur && ( | ||
| <ChartTooltip | ||
| cursor | ||
| content={ | ||
| <ChartTooltipContent | ||
| indicator="dot" | ||
| yAxisFormatter={yAxis?.tickFormatter} | ||
| /> | ||
| } | ||
| /> | ||
| )} |
There was a problem hiding this comment.
I think we're pushing too much logic on the charts. I would totally rethink this "blur" thing and try to find a higher-order solution (maybe even a higher-order component / wrapper). It can easily pollute the whole codebase.
There was a problem hiding this comment.
let's then start rethinking it until it is too late
There was a problem hiding this comment.
You mean "before it's too late" because "until it's too late" it's what we want to avoid 😆
There was a problem hiding this comment.
There was a problem hiding this comment.
If we wanna blur the YAxis, then the chart needs to know if it should be blurred, right? How else will be blur it? Adding an overlay doesn't make a lot of sense and the alternative would be passing down the className prop which would give too much control of the UI. I understand the concern, I just don't see a better option. It makes sense to me that the chart knows how to blur itself.
There was a problem hiding this comment.
Creating a and then letting the card act upon its state would help us avoid that blur property in the contract and also encourage a global solution. What do you think?
🔑 What?
Do not show tooltip when we are blurring the chart.
🚪 Why?
We shouldn't show any tooltip when hovering the chart in case it's blurred.
👁️ Visual proof
Screen.Recording.2024-10-01.at.13.44.14.mov