Copilot commented on code in PR #36793:
URL: https://github.com/apache/superset/pull/36793#discussion_r2658760652
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/Polygon.tsx:
##########
@@ -212,7 +212,7 @@ export const getLayer: GetLayerType<PolygonLayer> =
function ({
getPolygon: getPointsFromPolygon,
getFillColor: colorScaler,
getLineColor: sc ? [sc.r, sc.g, sc.b, 255 * sc.a] : undefined,
- getLineWidth: fd.line_width,
+ lineWidthMinPixels: fd.stroked ? fd.line_width : 0,
Review Comment:
The property name has been changed from `getLineWidth` to
`lineWidthMinPixels`, which is incorrect. The `lineWidthMinPixels` property
sets the minimum line width in pixels and doesn't replace `getLineWidth`.
According to the deck.gl PolygonLayer API and consistent with other layers
in this codebase (see Geojson and Contour layers), the correct property is
`getLineWidth`. Additionally, using `lineWidthMinPixels` doesn't respect the
`lineWidthUnits` setting on line 217.
The fix should conditionally set `getLineWidth` based on the `stroked` flag,
for example: `getLineWidth: fd.stroked ? fd.line_width : 0`
```suggestion
getLineWidth: fd.stroked ? fd.line_width : 0,
```
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Polygon/Polygon.tsx:
##########
@@ -212,7 +212,7 @@ export const getLayer: GetLayerType<PolygonLayer> =
function ({
getPolygon: getPointsFromPolygon,
getFillColor: colorScaler,
getLineColor: sc ? [sc.r, sc.g, sc.b, 255 * sc.a] : undefined,
- getLineWidth: fd.line_width,
+ lineWidthMinPixels: fd.stroked ? fd.line_width : 0,
Review Comment:
The behavior change to respect the `stroked` toggle when applying line width
lacks test coverage. Consider adding a test that verifies the layer
configuration when `stroked` is true vs false, ensuring that line width is only
applied when `stroked` is enabled.
--
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]
