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: make data tables support html #24368

Merged
merged 14 commits into from
Jun 14, 2023
Merged

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Jun 12, 2023

SUMMARY

allowing links and other basic HTML to render in data tables safely. This tackles:

  • in SQL Lab
  • in the Explore south pane in Results
  • in the Explore south pane in Samples
  • in "drill to details" menu picker (removes html tags if any)
  • in the table view of Drill to details

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2023-06-12 at 2 23 41 PM Screen Shot 2023-06-12 at 2 23 26 PM Screen Shot 2023-06-12 at 5 44 15 PM Screen Shot 2023-06-12 at 5 48 58 PM

TESTING INSTRUCTIONS

  • create an expression or calculated dim
  • render in SQL Lab, samples, result set view

ADDITIONAL INFORMATION

@mistercrunch mistercrunch marked this pull request as ready for review June 13, 2023 05:07
@worker24h
Copy link

worker24h commented Jun 14, 2023

The feature is very best,I need it too.

But, I hope to that, Add a config item by Web UI ,rather than CONCAT In sql

What are your ideas ? @mistercrunch

image

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Merging #24368 (009951e) into master (a3aacf2) will decrease coverage by 0.06%.
The diff coverage is 93.24%.

❗ Current head 009951e differs from pull request most recent head f94dd98. Consider uploading reports for the commit f94dd98 to get more accurate results

@@            Coverage Diff             @@
##           master   #24368      +/-   ##
==========================================
- Coverage   69.05%   68.99%   -0.06%     
==========================================
  Files        1903     1904       +1     
  Lines       74530    74144     -386     
  Branches     8105     8120      +15     
==========================================
- Hits        51464    51157     -307     
+ Misses      20955    20875      -80     
- Partials     2111     2112       +1     
Flag Coverage Δ
javascript 55.65% <90.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...acy-preset-chart-deckgl/src/components/Tooltip.tsx 0.00% <0.00%> (ø)
...lugins/plugin-chart-table/src/utils/formatValue.ts 66.66% <0.00%> (-3.93%) ⬇️
...c/components/Chart/DrillDetail/DrillDetailPane.tsx 75.32% <ø> (ø)
...end/src/components/Datasource/DatasourceEditor.jsx 65.35% <ø> (ø)
...-frontend/src/components/TableCollection/index.tsx 100.00% <ø> (ø)
superset-frontend/src/dashboard/actions/hydrate.js 2.04% <ø> (ø)
...mponents/DataTablesPane/components/SamplesPane.tsx 97.67% <ø> (ø)
...ataTablesPane/components/SingleQueryResultPane.tsx 85.71% <ø> (ø)
...erset-frontend/src/profile/components/fixtures.tsx 100.00% <ø> (ø)
superset/models/sql_lab.py 79.51% <0.00%> (+0.94%) ⬆️
... and 16 more

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

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

🎉

@rusackas
Copy link
Member

The feature is very best,I need it too.

But, I hope to that, Add a config item by Web UI ,rather than CONCAT In sql

What are your ideas ? @mistercrunch

image

I think the concat is nice in that it allows you to re-use the columnar data anywhere needed in the link. For example, a Github issue ID is used as the linked text AND a particular spot in the URL. If we were to add a link to the UI, we would need some form of syntax (e.g. handlebars/jinja) that would allow you to construct the link in the correct way... and that can have security-related concerns.

I love the idea of it being in the UI, but I'm having a hard time thinking of a way to do it that's both simple and secure.

@mistercrunch mistercrunch merged commit d2b0b8e into apache:master Jun 14, 2023
@mistercrunch mistercrunch deleted the sanitize_html branch June 14, 2023 22:54
@nytai
Copy link
Member

nytai commented Jun 14, 2023

Hmm, I was just looking into adding this support to the pivot tables too.

@worker24h
Copy link

The feature is very best,I need it too.
But, I hope to that, Add a config item by Web UI ,rather than CONCAT In sql
What are your ideas ? @mistercrunch
image

I think the concat is nice in that it allows you to re-use the columnar data anywhere needed in the link. For example, a Github issue ID is used as the linked text AND a particular spot in the URL. If we were to add a link to the UI, we would need some form of syntax (e.g. handlebars/jinja) that would allow you to construct the link in the correct way... and that can have security-related concerns.

I love the idea of it being in the UI, but I'm having a hard time thinking of a way to do it that's both simple and secure.

YES, I got it, thanks

@alexandrafetterman
Copy link

Will this work in just regular Table charts? Like by applying custom SQL to fields I have in the table using concatenate? Or just SQLlab?

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants