Skip to content
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

feat(datapoints-graph): added export to csv #20

Closed
wants to merge 10 commits into from

Conversation

janhommes
Copy link
Collaborator

@janhommes janhommes commented Oct 18, 2023

Proposed changes

Adds a CSV export to the datapoints view

  • new button for export
  • new modal for defining the filename and separator of the export
  • new configuration to export

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#18

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

package.json Outdated Show resolved Hide resolved
src/datapoints-graph/charts/charts.component.ts Outdated Show resolved Hide resolved
src/datapoints-graph/charts/charts.component.ts Outdated Show resolved Hide resolved
"jest-fail-on-console": "^3.1.1",
"jest-preset-angular": "^12.2.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"jest-preset-angular": "^12.2.0",
"jest-preset-angular": "^12.2.0",
"@types/file-saver": "^2.0.7",

Comment on lines +32 to +44
this.isLoading = true;
import('file-saver').then((fileSaver) => {
for (const series of this.csvData) {
const dataSet = series.data
.map((value: string[]) => value.join(this.form.separator))
.join('\n');
const blob = new Blob([dataSet], { type: 'text/csv;charset=utf-8' });
fileSaver.default.saveAs(blob, `${this.form.fileName}${series.id}.csv`);
}

this.isLoading = false;
this.bsModalRef.hide();
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.isLoading = true;
import('file-saver').then((fileSaver) => {
for (const series of this.csvData) {
const dataSet = series.data
.map((value: string[]) => value.join(this.form.separator))
.join('\n');
const blob = new Blob([dataSet], { type: 'text/csv;charset=utf-8' });
fileSaver.default.saveAs(blob, `${this.form.fileName}${series.id}.csv`);
}
this.isLoading = false;
this.bsModalRef.hide();
});
import { saveAs } from 'file-saver';
...
this.isLoading = true;
for (const series of this.csvData) {
const dataSet = series.data
.map((value: string[]) => value.join(this.form.separator))
.join('\n');
const blob = new Blob([dataSet], { type: 'text/csv;charset=utf-8' });
saveAs(blob, `${this.form.fileName}${series.id}.csv`);
}
this.isLoading = false;
this.bsModalRef.hide();

Copy link
Collaborator

@jdre-c8y jdre-c8y Nov 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why using import in method (as inline method, in method)? package file-saver is rather lightweightm, does it need lazy loading? also, after adding types, it's usage is more predictable (e.g. I immediately know that it is synchronous, but I was not sure before)

@jdre-c8y
Copy link
Collaborator

also I found a little issue on chrome- when we try to download multiple files at once (our case- I try to download csv of two series in data points graph), chrome blocks it and it's easy to miss that second file download was blocked. not sure if it's possible to handle as it's browser limitation, but if there is- it's worth to make it more user friendly.

@janhommes janhommes closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants