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

Remove xlsx export from CLI's interactive tables #6439

Merged
merged 10 commits into from
May 21, 2024

Conversation

piiq
Copy link
Contributor

@piiq piiq commented May 20, 2024

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:

  • remove posthog from template html of interactive tables
  • fix deploy script
  • update css to include better word wrap
  • update the table.html in charting extension

@piiq piiq added bug Fix bug dependencies Work related to dependencies security cli OpenBB Platform CLI P1 labels May 20, 2024
@piiq piiq requested a review from hjoaquim May 20, 2024 13:35
@piiq piiq self-assigned this May 20, 2024
Copy link
Contributor

@hjoaquim hjoaquim left a comment

Choose a reason for hiding this comment

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

If it's a table, I still have the options to download it as an Excel file (and it works):
image

@github-actions github-actions bot added platform OpenBB Platform v4 PRs for v4 labels May 21, 2024
@piiq
Copy link
Contributor Author

piiq commented May 21, 2024

@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.

Copy link
Contributor

@hjoaquim hjoaquim left a 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.

@IgorWounds
Copy link
Contributor

Tested this and it works and performs well! Thanks @piiq !

@IgorWounds IgorWounds enabled auto-merge May 21, 2024 12:02
@IgorWounds IgorWounds added this pull request to the merge queue May 21, 2024
@piiq piiq removed this pull request from the merge queue due to a manual request May 21, 2024
@piiq piiq merged commit c4de0a8 into develop May 21, 2024
9 checks passed
@piiq piiq deleted the bugfix/remove-xlsx-export-from-cli-tables branch May 21, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fix bug cli OpenBB Platform CLI dependencies Work related to dependencies P1 platform OpenBB Platform security v4 PRs for v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants