search

ESI-FAR / INA-tool

A tool for visualising and analysing Institutional Statements
MIT License
2 stars 0 forks source link

add csv download button #112

Open ermannolocascio opened 1 week ago

ermannolocascio commented 1 week ago

fix #106

ermannolocascio commented 1 week ago

hi guys, I have just implemented few changes which should address your observations (see last commit) - thank you. However, I have noticed two problems. Precisely, if I download the .csv file (columns and rows only) and I try to upload the same file in a second moment, all the connections are lost (problem A). Also, the Or Else column is named Extra Column in the INA.columnNames variable: this generates an error during the uploading phase (problem B).

For problem A, I guess we should clarify first what's the purpose of this csv download button. If the downloaded file is needed to resume the project in a second moment, then we should elaborate a solution that does not loose the info relative to the connections between statements.

For problem B, @sjvrijn please, do you remember why the column is renamed at some point in the process?

Please, let me know your idea @suvayu and @sjvrijn @OleMussmann / Cheers :)

sjvrijn commented 1 week ago

To me, A is not a problem, since I'm expecting the CSV download to only download the data in the table, which does not include the connections. I also wouldn't know how to nicely include that in the CSV anyway. For remembering the connections, we came up with the project .json download instead. We will have to make sure that it's clear which download option has which functionality, to not confuse people expecting connections to be remembered when downloading the table.

suvayu commented 1 week ago

We will have to make sure that it's clear which download option has which functionality, to not confuse people expecting connections to be remembered when downloading the table.

Why shouldn't we always download everything (i.e. project + data)?

ermannolocascio commented 1 week ago

Thanks guys. Then, let's keep it as it is and wait for a feedback from the users as discussed during the meeting.

@sjvrijn - should we plan a meeting with the users? I can do that, or, if you prefer, you do that. Ups to you. Cheers e

ermannolocascio commented 1 day ago

Hi @sjvrijn @suvayu , should we temporarly merge this PR? I think once we get a sort of UI feedback from the users then we move the button elsewhere. Cheers, E

suvayu commented 1 day ago

As long as you create an issue documenting the compromise (or unresolved issue), I think it should be okay to merge.

EDIT: sorry haven't looked at anything besides Mopo for a while :-p