TekMonksGitHub / monkvision

Dashboarding framework
Other
1 stars 10 forks source link

Export CSV feature #23

Closed sridharan-dlt closed 3 years ago

sridharan-dlt commented 3 years ago

PR overview

Functionality:

PS: As per discussion on PR #19, Export CSV functionality is re-worked using the front-end. Now the data which is visible on the dashboard is being generated as a CSV file.

sridharan-dlt commented 3 years ago

@TekMonksGitHub could you please review this PR.

TekMonksGitHub commented 3 years ago

Few things are wrong

1 - Why do we need a menu? It should be a simple image and clicking it exports the CSV. So this seems wrong

image

2 - I don't understand this either - it should be a simple onclick - call a function in the component to export the CSV and download it

image

No hash should ever be needed. In fact, 99% of times hashing to trap events is just plain wrong! HTML elements have onclick for a reason!

3 - When click is done there would be a single function which will take the chart's data, and export it to CSV file. Yes you will need an "" element but this is done dynamically, in fact, all you do is document.createElement("a").. etc within that function itself, and then click it. One doesn't go ahead creating physical elements etc.

And many other things are wrong! Please recode.

A simple image with a onclick handler sending a simple function call to the component which uses component's memory (see Monkshu component's memory) to get the data which was used to render the chart, and then simply exports it. This is probably going to be a 10 lines of code in a single function and that is it!

I am rejecting this fix. Design and code are wrong and fundamentals are wrong.