codeant-ai-for-open-source[bot] commented on code in PR #36936:
URL: https://github.com/apache/superset/pull/36936#discussion_r2666834875
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -856,34 +854,20 @@ const ResultSet = ({
}
}
- let progressBar;
- if (query.progress > 0) {
- progressBar = (
- <ProgressBar percent={parseInt(query.progress.toFixed(0), 10)} striped />
- );
- }
-
const progressMsg = query?.extra?.progress ?? null;
return (
- <>
- <ResultlessStyles>
- <div>{!progressBar && <Loading position="normal" />}</div>
- {/* show loading bar whenever progress bar is completed but needs time
to render */}
- <div>{query.progress === 100 && <Loading position="normal" />}</div>
- <QueryStateLabel query={query} />
- <div>
- {progressMsg && <Alert type="success" message={progressMsg} />}
- </div>
- <div>{query.progress !== 100 && progressBar}</div>
- {trackingUrl && <div>{trackingUrl}</div>}
- </ResultlessStyles>
+ <ResultlessStyles>
+ {progressMsg && (
+ <Alert type="success" message={progressMsg} closable={false} />
+ )}
+ {trackingUrl && <div>{trackingUrl}</div>}
Review Comment:
**Suggestion:** Rendering `trackingUrl` wrapped inside an extra `<div>` may
change styling or semantics of the button element stored in `trackingUrl` (it
is a JSX Button element); render the JSX element directly to preserve its
attributes and styling instead of nesting it inside a new container. [possible
bug]
**Severity Level:** Critical 🚨
```suggestion
{trackingUrl && trackingUrl}
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
trackingUrl is a JSX Button element. Wrapping it in an extra div can change
layout/styling and is inconsistent
with other places where trackingUrl is rendered directly. Rendering the
element directly preserves attributes
and styling.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/ResultSet/index.tsx
**Line:** 864:864
**Comment:**
*Possible Bug: Rendering `trackingUrl` wrapped inside an extra `<div>`
may change styling or semantics of the button element stored in `trackingUrl`
(it is a JSX Button element); render the JSX element directly to preserve its
attributes and styling instead of nesting it inside a new container.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
