villebro commented on code in PR #23698:
URL: https://github.com/apache/superset/pull/23698#discussion_r1173343609
##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js:
##########
@@ -187,6 +192,10 @@ function WorldMap(element, props) {
col: entity,
op: '==',
val,
+ formattedVal:
+ getColumnLabel(entity) === formData.granularitySqla
+ ? String(timestampFormatter(val))
+ : String(val),
Review Comment:
While this will probably work well in legacy charts that still have a time
section, this won't work for "modern" charts that don't have a time section
when `GENERIC_CHART_AXES` is enabled. For this chart this might be the correct
solution, as I don't remember if `viz.py` returns `colnames` (=all columns) and
`coltypes` (=the respective datatypes for said columns), but at least for
charts that are powered by the v1 chart API we should check if the datatype of
the column is `GenericDataType.TEMPORAL`, and if so, format it with
`timestampFormatter`.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx:
##########
@@ -167,6 +171,10 @@ export default function EchartsMixedTimeseries({
col: dimension,
op: '==',
val: values[i],
+ formattedVal:
+ DTTM_ALIAS === getColumnLabel(dimension)
+ ? String(timestampFormatter(values[i]))
+ : String(values[i]),
Review Comment:
same here, `DTTM_ALIAS` is already mostly deprecated, so we should refrain
from using it in these types of checks. Again, this might be the correct
solution for legacy charts that are powered by `viz.py`, but in the ECharts
plugin we should not be using `DTTM_ALIAS` anymore.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
