Copilot commented on code in PR #53087:
URL: https://github.com/apache/spark/pull/53087#discussion_r2536129205
##########
core/src/main/resources/org/apache/spark/ui/static/spark-dag-viz.js:
##########
@@ -157,7 +157,10 @@ function renderDagViz(forJob) {
}
// Render
- var svg = graphContainer().append("svg").attr("class", jobOrStage);
+ const svg = graphContainer()
+ .append("svg")
+ .attr("class", jobOrStage)
+ .attr("viewBox", `${-VizConstants.svgMarginX} ${-VizConstants.svgMarginY}
${window.screen.availWidth} 1000`);
Review Comment:
The hardcoded value `1000` should be defined as a named constant in
`VizConstants` (lines 53-60) for better maintainability. Consider adding
something like `initialViewBoxHeight: 1000` to the VizConstants object and
referencing it here.
Example:
```javascript
.attr("viewBox", `${-VizConstants.svgMarginX} ${-VizConstants.svgMarginY}
${window.screen.availWidth} ${VizConstants.initialViewBoxHeight}`);
```
##########
core/src/main/resources/org/apache/spark/ui/static/spark-dag-viz.js:
##########
@@ -157,7 +157,10 @@ function renderDagViz(forJob) {
}
// Render
- var svg = graphContainer().append("svg").attr("class", jobOrStage);
+ const svg = graphContainer()
+ .append("svg")
+ .attr("class", jobOrStage)
+ .attr("viewBox", `${-VizConstants.svgMarginX} ${-VizConstants.svgMarginY}
${window.screen.availWidth} 1000`);
Review Comment:
Using `window.screen.availWidth` can cause issues in server-side rendering,
headless browsers, or testing environments where `window.screen` may not be
available or reliable. Consider using a more robust approach like:
- `window.innerWidth` or `document.documentElement.clientWidth` for
viewport-based sizing
- Or a safe default constant since `resizeSvg()` adjusts this value anyway
at line 427
Example:
```javascript
.attr("viewBox", `${-VizConstants.svgMarginX} ${-VizConstants.svgMarginY}
${window.innerWidth || 1920} 1000`);
```
```suggestion
.attr("viewBox", `${-VizConstants.svgMarginX}
${-VizConstants.svgMarginY} ${window.innerWidth || 1920} 1000`);
```
--
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]
