Copilot commented on code in PR #37037:
URL: https://github.com/apache/superset/pull/37037#discussion_r2692098479
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -502,7 +502,8 @@ const AlertReportModal:
FunctionComponent<AlertReportModalProps> = ({
const [isScreenshot, setIsScreenshot] = useState<boolean>(false);
useEffect(() => {
- setIsScreenshot(reportFormat === 'PNG');
+ const fmt = String(reportFormat || '').trim().toUpperCase();
+ setIsScreenshot(fmt === 'PNG' || fmt === 'PDF');
Review Comment:
The string normalization with `String(...).trim().toUpperCase()` is
unnecessary. The `reportFormat` values come from the `FORMAT_OPTIONS` object
which defines them as uppercase strings ('PNG', 'PDF', 'CSV', 'TEXT'), and
`setReportFormat` is only called with these predefined values. The original
simple comparison `reportFormat === 'PNG'` pattern should be sufficient.
Simplify to: `setIsScreenshot(reportFormat === 'PNG' || reportFormat ===
'PDF');`
```suggestion
setIsScreenshot(reportFormat === 'PNG' || reportFormat === 'PDF');
```
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -502,7 +502,8 @@ const AlertReportModal:
FunctionComponent<AlertReportModalProps> = ({
const [isScreenshot, setIsScreenshot] = useState<boolean>(false);
useEffect(() => {
- setIsScreenshot(reportFormat === 'PNG');
+ const fmt = String(reportFormat || '').trim().toUpperCase();
+ setIsScreenshot(fmt === 'PNG' || fmt === 'PDF');
Review Comment:
The new behavior allowing screenshot width for PDF format lacks test
coverage. Consider adding a test case that verifies the screenshot width field
is visible when PDF format is selected, similar to existing tests for PNG
format.
--
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]
