Refactor charts code #1169
Refactor charts code #1169
Conversation
|
Please review @ericboucher |
Bhavyajain21
changed the title
|
@Bhavyajain21 Thanks for your work, could you make sure that the linter and type checks are passing please? As a bonus this will create a QA deploy automatically. |
@ericboucher, yes I have been trying to do that but the approcahes haven't worked much from my side. |
|
Thanks for the commit @ericboucher |
|
@ericboucher Can you please review this? |
|
@Bhavyajain21 thanks for your work, I will try to finish my review soon but it's a bit hard to QA. |
| .map(row => row[config.category]); | ||
| }, [chartRange, config.category, header, indices, tableRows, transpose]); | ||
| const labels = useMemo(() => { | ||
| return transpose |
There was a problem hiding this comment.
@Bhavyajain21 in some cases in this PR you are changing to ternary tetss and in others you are changing a ternary to an if statement, can you explain the rationale behind these changes?
| }, [ | ||
| colors, | ||
| tableRows, |
There was a problem hiding this comment.
I see a lot of order changes in the dependency arrays, is there a specific reason or logic? If not, it makes the code changes a bit more noisy and should be avoided imho :)
ericboucher
changed the title
|
Hey @ericboucher, I've been busy from past few days. Sorry for the delayed response. I currently don't have my system with me. I need to review the code to get the reviews resolved. |
Description
This fixes #1111
/claim #1111
I've added two new useCallback hooks, handleDownloadChartPng and handleDownloadCsv, to handle the download of PNG and CSV files respectively. These hooks are more efficient because they have a stable reference, preventing unnecessary re-renders.
Additionally, the rendering logic for the chart component has been optimized. We only render either a Bar or Line chart based on the config.type property, avoiding unnecessary conditional rendering.
These changes aim to optimize performance and improve code readability.
While these optimizations are contributing to better load time balance between Chrome and Firefox, it's important to note that browser performance can be influenced by various factors beyond the control of the application code. Factors such as browser engine differences, hardware capabilities, network conditions, and browser extensions can all impact performance.