rusackas commented on code in PR #33247:
URL: https://github.com/apache/superset/pull/33247#discussion_r2801740387
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Geojson/Geojson.tsx:
##########
@@ -321,7 +321,9 @@ export const getLayer: GetLayerType<GeoJsonLayer> =
function ({
getFillColor(feature, filterState?.value),
getLineColor,
getLineWidth: fd.line_width || 1,
- pointRadiusScale: fd.point_radius_scale,
+ getPointRadius: fd.point_radius ?? 10,
+ pointRadiusUnits: fd.point_radius_units ?? 'pixels',
Review Comment:
Great catch! You're absolutely right about the backward compatibility issue.
Fixed in 7705ffcfe0 - the runtime fallbacks now use deck.gl's defaults
(`getPointRadius=1`, `pointRadiusUnits='meters'`) instead of the control panel
defaults. This way:
- **Existing charts** (without new fields) → get deck.gl defaults →
rendering unchanged
- **New charts** → get control panel defaults (`point_radius=10`,
`units='pixels'`) → improved UX
Added a comment in the code explaining the reasoning.
Regarding the suggested test - that's a good idea for ensuring this doesn't
regress. I'll leave that as a follow-up since this PR doesn't currently have
test coverage for the layer props, but it's worth adding.
--
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]
