You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/01/22 10:07:12 UTC
[GitHub] [superset] lilila opened a new issue #12681: Pivot Table Rendering Superset 1.0
lilila opened a new issue #12681:
URL: https://github.com/apache/superset/issues/12681
I have an issue with pivot table I didn't have using version 0.38.
I use to display links (either links or images) in table cells. It always worked in simple Table and it is still working.
With the previous version of superset I was able to display images as well within Pivot Table (it was necessary to add `escape = False` in df.to_html() in superset/viz.py)
With the current version (1.0) if if print what is returned in viz.py I have :
```
<tr>
<th>3</th>
<td><a href="https://www.allocine.fr/film/fichefilm_gen_cfilm=275832.html" target="_blank"><img src="http://fr.web.img4.acsta.net/pictures/20/08/05/09/21/0019416.jpg", height=60></a></td>
<td>263184</td>
<td><a href="http://www.allocine.fr/film/fichefilm_gen_cfilm=275832.html" target="_blank">ANTEBELLUM</a></td>
</tr>
```
If I look at what is rendered in the chart, I have the following
```
<tr>
<th>3</th>
<td></td>
<td>263k</td>
<td>ANTEBELLUM</td>
</tr>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777312574
@ktmud I did actually test this with `escape=False` and it does work with @mayurnewase 's modification. I agree it's problematic to maintain support for customizations that aren't relevant for the current vanilla codebase, but I'm ok making this change for now to unblock orgs that have this need as I don't see any direct downsides to it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777209747
Hi @lilila thanks for raising this.
I have raised pr to handle image rendering,but I think setting escape to False might not be a good idea, so you might need to continue doing it manually.
To display text with links would be bit difficult to handle as it will require html parsing and finding correct text inside and setting it seperately along with innerHtml property.
If this is absolute requirement it can be done at the cost of number,date,null formatting,by applying following patch in superset-ui repository and linking that plugin to main repo.
```
diff --git a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
index ec822dbc..97cd62e9 100644
--- a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
+++ b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
@@ -117,7 +117,7 @@ function PivotTable(element, props) {
dateRegex,
dateFormatter,
);
- $(this)[0].textContent = textContent;
+ //$(this)[0].textContent = textContent;
$(this).attr = attr;
}
});
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777349725
I guess one downside is if the fix is not clean, more hacks would mean more future maintenance cost and wasted work (especially if we are rewriting this soon anyway).
Luckily, a complete fix seems to be fairly direct in this case: https://github.com/apache-superset/superset-ui/pull/954#issuecomment-777310929 So I'm OK with pushing this forward.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777209747
Hi @lilila thanks for raising this.
I have raised pr to handle image rendering(see https://github.com/apache-superset/superset-ui/pull/954),but I think setting escape to False might not be a good idea, so you might need to continue doing it manually.
To display text with links would be bit difficult to handle as it will require html parsing and finding correct text inside and setting it seperately along with innerHtml property.
If this is absolute requirement it can be done at the cost of number,date,null formatting,by applying following patch in superset-ui repository and linking that plugin to main repo.
```
diff --git a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
index ec822dbc..97cd62e9 100644
--- a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
+++ b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
@@ -117,7 +117,7 @@ function PivotTable(element, props) {
dateRegex,
dateFormatter,
);
- $(this)[0].textContent = textContent;
+ //$(this)[0].textContent = textContent;
$(this).attr = attr;
}
});
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777209747
Hi @lilila thanks for raising this.
I have raised pr to handle image rendering(see https://github.com/apache-superset/superset-ui/pull/954), but I think setting escape to False might not be a good idea for security reasons, so you might need to continue doing it manually.
To display text with links would be bit difficult to handle as it will require html parsing and finding correct text inside and setting it seperately along with innerHtml property.
If this is absolute requirement it can be done at the cost of number,date,null formatting,by applying following patch in superset-ui repository and linking that plugin to main repo.
```
diff --git a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
index ec822dbc..97cd62e9 100644
--- a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
+++ b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
@@ -117,7 +117,7 @@ function PivotTable(element, props) {
dateRegex,
dateFormatter,
);
- $(this)[0].textContent = textContent;
+ //$(this)[0].textContent = textContent;
$(this).attr = attr;
}
});
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777210187
@junlincc what do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc closed issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
junlincc closed issue #12681:
URL: https://github.com/apache/superset/issues/12681
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777209747
Hi @lilila thanks for raising this.
I have raised pr to handle image rendering(see https://github.com/apache-superset/superset-ui/pull/954), but I think setting escape to False might not be a good idea, so you might need to continue doing it manually.
To display text with links would be bit difficult to handle as it will require html parsing and finding correct text inside and setting it seperately along with innerHtml property.
If this is absolute requirement it can be done at the cost of number,date,null formatting,by applying following patch in superset-ui repository and linking that plugin to main repo.
```
diff --git a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
index ec822dbc..97cd62e9 100644
--- a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
+++ b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
@@ -117,7 +117,7 @@ function PivotTable(element, props) {
dateRegex,
dateFormatter,
);
- $(this)[0].textContent = textContent;
+ //$(this)[0].textContent = textContent;
$(this).attr = attr;
}
});
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro closed issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
villebro closed issue #12681:
URL: https://github.com/apache/superset/issues/12681
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-765498066
Thank you Lilila for reporting the issue, we confirm that it has been resolved in latest master by https://github.com/apache/superset/pull/12564. we tagged the fix `v1.0.1`, which will be available to users soon. 🙏
@mayurnewase thank you for your fix!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mayurnewase edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
mayurnewase edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777209747
Hi @lilila thanks for raising this.
I have raised pr to handle image rendering(see https://github.com/apache-superset/superset-ui/pull/954), but I think setting escape to False may not be a good idea for security reasons, so you might need to continue doing it manually.
To display text with links would be bit difficult to handle as it will require html parsing and finding correct text inside and setting it seperately along with innerHtml property.
If this is absolute requirement it can be done at the cost of number,date,null formatting,by applying following patch in superset-ui repository and linking that plugin to main repo.
```
diff --git a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
index ec822dbc..97cd62e9 100644
--- a/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
+++ b/plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js
@@ -117,7 +117,7 @@ function PivotTable(element, props) {
dateRegex,
dateFormatter,
);
- $(this)[0].textContent = textContent;
+ //$(this)[0].textContent = textContent;
$(this).attr = attr;
}
});
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
junlincc commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-775688340
Hi Lise, sorry we misunderstood the issue. the PR i mentioned didn't cover this case🤦🏾♀️. we are looking into it and will push a fix soon! I apologize.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-777309259
My guess is, without `escape=False`, even `<img>` wouldn't render, `textContent` will just return `<img ... >`.
As this hack (manual modification of `superset/viz.py`) is not officially supported by Superset, I'd be hesitant to add more hacks to support it (or even categorize this as a regression).
If we want to properly support rendering (limited) HTML in Pivot Table cells, we should start thinking of migrating it to the new API or at least move the HTML rendering to the front-end.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-765498066
Thank you Lilila for reporting the issue, we confirm that it has been resolved in latest master by https://github.com/apache/superset/pull/12564 and https://github.com/apache-superset/superset-ui/pull/880. we tagged the fix `v1.0.1`, which will be available to users soon. 🙏
@mayurnewase thank you for your fix!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc edited a comment on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-775688340
Hi Lise, sorry we misunderstood the issue. the PR i mentioned didn't cover this case🤦🏾♀️. we are looking into it and will push a fix soon! I apologize. @lilila
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lilila commented on issue #12681: [chart]Pivot Table Rendering Superset 1.0
Posted by GitBox <gi...@apache.org>.
lilila commented on issue #12681:
URL: https://github.com/apache/superset/issues/12681#issuecomment-775675762
Dear @junlincc ,
I pulled the 1.0.1 version and I still have empty cells where I should see images in Pivot Table. Do you have any suggestion?
Best,
Lise
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org