Copilot commented on code in PR #36201:
URL: https://github.com/apache/superset/pull/36201#discussion_r2547196111


##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Geojson/controlPanel.ts:
##########
@@ -63,6 +82,221 @@ const config: ControlPanelConfig = {
         [fillColorPicker, strokeColorPicker],
         [filled, stroked],
         [extruded],
+        [
+          {
+            name: 'enable_labels',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Enable labels'),
+              description: t('Enables rendering of labels for GeoJSON points'),
+              default: false,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'enable_label_javascript_mode',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Enable label JavaScript mode'),
+              description: t(
+                'Enables custom label configuration via JavaScript',
+              ),
+              visibility: ({ form_data }) => !!form_data.enable_labels,
+              default: false,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'label_property_name',
+            config: {
+              type: 'TextControl',
+              label: t('Label property name'),
+              description: t('The feature property to use for point labels'),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_labels &&
+                !form_data.enable_label_javascript_mode,
+              default: 'name',
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'label_color',
+            config: {
+              type: 'ColorPickerControl',
+              label: t('Label color'),
+              description: t('The color of the point labels'),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_labels &&
+                !form_data.enable_label_javascript_mode,
+              default: BLACK_COLOR,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'label_size',
+            config: {
+              type: 'SelectControl',
+              freeForm: true,
+              label: t('Label size'),
+              description: t('The font size of the point labels'),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_labels &&
+                !form_data.enable_label_javascript_mode,
+              validators: [legacyValidateInteger],
+              choices: formatSelectOptions([8, 16, 24, 32, 64, 128]),
+              default: 24,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'label_size_unit',
+            config: {
+              type: 'SelectControl',
+              label: t('Label size unit'),
+              description: t('The unit for label size'),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_labels &&
+                !form_data.enable_label_javascript_mode,
+              choices: [
+                ['meters', t('Meters')],
+                ['pixels', t('Pixels')],
+              ],
+              default: 'pixels',
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'label_javascript_config_generator',
+            config: {
+              ...jsFunctionControl(
+                t('Label JavaScript config generator'),
+                t(
+                  'A JavaScript function that generates a label configuration 
object',
+                ),
+                undefined,
+                undefined,
+                defaultLabelConfigGenerator,
+              ),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_labels &&
+                !!form_data.enable_label_javascript_mode,
+            },
+          },
+        ],
+        [
+          {
+            name: 'enable_icons',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Enable icons'),
+              description: t('Enables rendering of icons for GeoJSON points'),
+              default: false,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'enable_icon_javascript_mode',
+            config: {
+              type: 'CheckboxControl',
+              label: t('Enable icon JavaScript mode'),
+              description: t(
+                'Enables custom icon configuration via JavaScript',
+              ),
+              visibility: ({ form_data }) => !!form_data.enable_icons,
+              default: false,
+              renderTrigger: true,
+            },
+          },
+        ],
+        [
+          {
+            name: 'icon_url',
+            config: {
+              type: 'TextControl',
+              label: t('Icon URL'),
+              description: t(
+                'The image URL of the icon to display for GeoJSON points. ' +
+                  'Note that the image URL must conform to the content ' +
+                  'security policy (CSP) in order to load correctly.',
+              ),
+              visibility: ({ form_data }) =>
+                !!form_data.enable_icons &&
+                !form_data.enable_icon_javascript_mode,
+              default: '',

Review Comment:
   The `icon_url` field has a default value of an empty string. When icons are 
enabled but no URL is provided, this will cause icons to fail to load silently. 
Consider either:
   1. Making this field required when icons are enabled (by adding a validator)
   2. Providing a more helpful default icon URL
   3. Adding validation that shows an error when icons are enabled but the URL 
is empty
   
   This would provide better user experience by alerting users that they need 
to provide a valid icon URL.



-- 
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]

Reply via email to