-
Notifications
You must be signed in to change notification settings - Fork 272
fix(plugin-chart-echarts): sanitize series from html tags #1126
fix(plugin-chart-echarts): sanitize series from html tags #1126
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/superset/superset-ui/FjicMRQoqHWqZ8qzxg5nruoC2hhm |
Codecov Report
@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
+ Coverage 29.00% 29.03% +0.02%
==========================================
Files 463 463
Lines 9339 9342 +3
Branches 1494 1494
==========================================
+ Hits 2709 2712 +3
Misses 6418 6418
Partials 212 212
Continue to review full report at Codecov.
|
@villebro great job! A small suggestion, maybe we can sanitize some general string like |
Yes, much cleaner approach. I'll update accordingly 👍 |
f9f2f67
to
5033fc8
Compare
export function sanitizeHtml(text: string): string { | ||
return format.encodeHTML(text); | ||
} |
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.
@stephenLYZ I left the original sanitizeHtml
function in place so we can more easily swap encodeHTML
out if needed later.
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.
LGTM
🐛 Bug Fix
Remove html tags from series names in tooltip.
BEFORE
Currently a series names that could be html tags don't show correctly on the tooltip(the series name
<NULL>
which can be seen on the legend does not show):AFTER
After they show correctly (legend omitted in screenshot):