korbit-ai[bot] commented on code in PR #34507:
URL: https://github.com/apache/superset/pull/34507#discussion_r2254652990
##########
superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx:
##########
@@ -149,7 +150,7 @@ function TableCollection<T extends object>({
size={size}
data-test="listview-table"
pagination={false}
- tableLayout="auto"
+ tableLayout="fixed"
Review Comment:
### Table Layout Change May Cause Content Truncation <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Changing table layout to 'fixed' while adding text overflow properties may
cause content truncation in table cells without proper width specifications.
###### Why this matters
Fixed table layout with ellipsis overflow can cause important content to be
hidden from users if column widths are not explicitly defined, leading to poor
user experience and potential data visibility issues.
###### Suggested change ∙ *Feature Preview*
Either add explicit width definitions for columns or maintain the 'auto'
layout. If fixed layout is required, ensure column widths are properly defined:
```tsx
const mappedColumns = mapColumns<T>(
columns.map(col => ({
...col,
width: col.width || 150, // Set appropriate default width
})),
headerGroups,
columnsForWrapText,
);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/888a80f6-0e22-4d98-8105-9a7024646c2a/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/888a80f6-0e22-4d98-8105-9a7024646c2a?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/888a80f6-0e22-4d98-8105-9a7024646c2a?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/888a80f6-0e22-4d98-8105-9a7024646c2a?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/888a80f6-0e22-4d98-8105-9a7024646c2a)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a4fcadc3-e4fc-4c09-9061-f25e65998811 -->
[](a4fcadc3-e4fc-4c09-9061-f25e65998811)
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -40,23 +40,24 @@ export interface RunQueryActionButtonProps {
overlayCreateAsMenu: ReactElement | null;
}
-const buildText = (
+const buildTextAndIcon = (
shouldShowStopButton: boolean,
selectedText: string | undefined,
theme: SupersetTheme,
-): string | JSX.Element => {
+): { text: string; icon?: IconType } => {
+ let text = t('Run');
+ let icon: IconType | undefined;
if (shouldShowStopButton) {
- return (
- <>
- <Icons.Square iconSize="xs" iconColor={theme.colorIcon} />
- {t('Stop')}
- </>
- );
+ text = t('Stop');
+ icon = <Icons.Square iconSize="xs" iconColor={theme.colorIcon} />;
}
if (selectedText) {
- return t('Run selection');
+ text = t('Run selection');
}
Review Comment:
### Incorrect button text priority logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The order of conditions allows 'Run selection' text to override 'Stop' text
when both shouldShowStopButton is true and selectedText exists.
###### Why this matters
This can confuse users when they try to stop a running query with selected
text, as the button will display 'Run selection' instead of 'Stop', potentially
leading to unintended query executions.
###### Suggested change ∙ *Feature Preview*
Reorder the conditions to ensure 'Stop' text takes precedence when
shouldShowStopButton is true:
```typescript
let text = t('Run');
let icon: IconType | undefined;
if (selectedText && !shouldShowStopButton) {
text = t('Run selection');
}
if (shouldShowStopButton) {
text = t('Stop');
icon = <Icons.Square iconSize="xs" iconColor={theme.colorIcon} />;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d817a5eb-3fa3-4d50-96a4-04deb797f121/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d817a5eb-3fa3-4d50-96a4-04deb797f121?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d817a5eb-3fa3-4d50-96a4-04deb797f121?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d817a5eb-3fa3-4d50-96a4-04deb797f121?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d817a5eb-3fa3-4d50-96a4-04deb797f121)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4dcdeee6-a05a-4245-bad9-3bb475168198 -->
[](4dcdeee6-a05a-4245-bad9-3bb475168198)
--
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]
