-
Notifications
You must be signed in to change notification settings - Fork 2
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: Add GridTable csv export. #1063
Conversation
@@ -3799,22 +3833,6 @@ function Collapse({ id }: { id: string }) { | |||
); | |||
} | |||
|
|||
function Select({ id }: { id: string }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was no longer used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This is a fairly small PR for something that seems like a bigger lift from the outside.
src/components/Table/GridTableApi.ts
Outdated
|
||
public copyToClipboard(): Promise<void> { | ||
// Copy the CSV content to the clipboard | ||
return navigator.clipboard.writeText(this.generateCsvContent().join("\n")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This can throw an error if copying to the clipboard is not allowed. I'm not sure how this would impact the rest of the application if the error is thrown (hopefully not take it down completely), but makes me wonder if we should wrap in a try/catch and report to DataDog and maybe a window.alert(e.message)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, afaiu it would return a rejected Promise, instead of immediately throwing, so I was thinking the caller could just deal with it -- but I suppose centralized error handling would be nice, so I added a q&d window.alert for now (but still returning a rejected promise, with the idea that the caller UI could show like a red "x" instead of a green "check").
In terms of logging, I think dd should be logging all of our rejected promises, which is what this would be atm... 🤔
const maybeContent = applyRowFn(column as GridColumnWithId<R>, row, rowApi, level, isExpanded, { | ||
|
||
// Combine all our drag stuff into a mini-context/parameter object... | ||
const dragData: DragData<R> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: Thanks for cleaning up
## [2.364.0](v2.363.0...v2.364.0) (2024-09-03) ### Features * Add GridTable csv export. ([#1063](#1063)) ([3d61e5d](3d61e5d))
🎉 This PR is included in version 2.364.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
As easy as:
Currently assumes all of the data is already available client-side -- not really sure how this would work with server-side driven pagination.
Also assumes we want all of the columns/rows included, and that they're directly mapped from the
content
/value
keys. We could also add acsvValue
to let columns provide CSV-specific values.