-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Remove xlsx export from CLI's interactive tables #6439
Conversation
Does this actually change the code in Plotly JS: |
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.
…-cli-tables' into bugfix/remove-xlsx-export-from-cli-tables
@hjoaquim I've added a few commits that update the built html in the charting library and the corresponding build script. I've also removed the posthog code and incorporated the css that @deeleeramone used in the template html into the index.css file. |
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.
Tables look good, and the export functionalities are working perfectly now. Building the openbb-charting
wheel and use it also works as expected.
Tested this and it works and performs well! Thanks @piiq ! |
This PR removes the option to export tables to excel from the UI of the interactive table.
The xlsx export in CLI's interactive tables relied on a vulnerable version of the xlsx npm package.
The SheetsJS project that is behind xlsx package does not distribute the latest packages via npm making it a bit harder to follow updated.
Since we have xlsx export functionality on the CLI and platform level and there is still an option to export the data from a the interactive table in machine readable format, I decided to remove the vulnerable dependency instead of trying to patch or find a replacement.
cc @jose-donato @tehcoderer
Additional changed following review: