sadpandajoe commented on code in PR #39760:
URL: https://github.com/apache/superset/pull/39760#discussion_r4401903823
##########
superset-frontend/plugins/plugin-chart-cartodiagram/src/util/layerUtil.tsx:
##########
@@ -158,6 +173,35 @@ export const createWfsLayer = async (wfsLayerConf:
WfsLayerConf) => {
});
};
+/**
+ * Create a DATA layer.
+ *
+ * @param dataLayerConf The layer configuration
+ *
+ * @returns The created DATA layer
+ */
+export const createDataLayer = async (dataLayerConf: DataLayerConf) => {
+ const { attribution, style } = dataLayerConf;
+ const dataSource = new VectorSource({
+ attributions: attribution,
Review Comment:
`createDataLayer` passes the creator-supplied `attribution` straight to
`VectorSource`, while the WMS, XYZ and WFS paths run it through
`escapeAttribution` because OpenLayers renders attributions via `innerHTML`. A
saved chart whose `layer_configs` contains `{"type":"DATA","attribution":"<img
src=x onerror=...>"}` would execute script in any viewer's browser when the
attribution control renders (the renderer picks DATA purely from `type`,
regardless of whether the Explore UI offers it). Should this use
`escapeAttribution(attribution)` like the other layer types?
##########
superset-frontend/plugins/plugin-chart-cartodiagram/src/components/OlChartMap.tsx:
##########
@@ -196,6 +325,10 @@ export const OlChartMap = (props: OlChartMapProps) => {
addLayers(layerConfigs);
}, [olMap, layerConfigs]);
+ useEffect(() => {
+ applyMapExtentView(olMap, currentMapMaxExtent);
Review Comment:
This replaces the map's `View` (via `olMap.setView(...)`) whenever the max
extent changes, but the zoom-sync effect further down (the
`view.on('change:resolution', ...)` subscription, deps `[olMap,
setControlValue, chartSize]`) bound to `olMap.getView()` once and is not re-run
for a new view. After picking CUSTOM or clicking "Use current extent", zooming
no longer reaches `onViewChange`, so `chart_size.configs.zoom` (and the
"Current Zoom" display / zoom-relative size editing) stays stale until
something else retriggers this effect. Should the view (or
`currentMapMaxExtent`) be part of this effect's dependencies?
--
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]
