Skip to content

Commit 463962a

Browse files
authored
fix(sqllab): misplaced limit warning alert (#25306)
1 parent 1716b9f commit 463962a

File tree

2 files changed

+97
-78
lines changed

2 files changed

+97
-78
lines changed

superset-frontend/src/SqlLab/components/ResultSet/index.tsx

+92-77
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ const ResultSet = ({
301301
return <div />;
302302
};
303303

304-
const renderRowsReturned = () => {
304+
const renderRowsReturned = (alertMessage: boolean) => {
305305
const { results, rows, queryLimit, limitingFactor } = query;
306306
let limitMessage = '';
307307
const limitReached = results?.displayLimitReached;
@@ -353,59 +353,70 @@ const ResultSet = ({
353353

354354
const tooltipText = `${rowsReturnedMessage}. ${limitMessage}`;
355355

356+
if (alertMessage) {
357+
return (
358+
<>
359+
{!limitReached && shouldUseDefaultDropdownAlert && (
360+
<div ref={calculateAlertRefHeight}>
361+
<Alert
362+
type="warning"
363+
message={t('%(rows)d rows returned', { rows })}
364+
onClose={() => setAlertIsOpen(false)}
365+
description={t(
366+
'The number of rows displayed is limited to %(rows)d by the dropdown.',
367+
{ rows },
368+
)}
369+
/>
370+
</div>
371+
)}
372+
{limitReached && (
373+
<div ref={calculateAlertRefHeight}>
374+
<Alert
375+
type="warning"
376+
onClose={() => setAlertIsOpen(false)}
377+
message={t('%(rows)d rows returned', { rows: rowsCount })}
378+
description={
379+
isAdmin
380+
? displayMaxRowsReachedMessage.withAdmin
381+
: displayMaxRowsReachedMessage.withoutAdmin
382+
}
383+
/>
384+
</div>
385+
)}
386+
</>
387+
);
388+
}
389+
const showRowsReturned =
390+
showSqlInline || (!limitReached && !shouldUseDefaultDropdownAlert);
391+
356392
return (
357-
<ReturnedRows>
358-
{!limitReached && !shouldUseDefaultDropdownAlert && (
359-
<Tooltip
360-
id="sqllab-rowcount-tooltip"
361-
title={tooltipText}
362-
placement="left"
363-
>
364-
<Label
365-
css={css`
366-
line-height: ${theme.typography.sizes.l}px;
367-
`}
393+
<>
394+
{showRowsReturned && (
395+
<ReturnedRows>
396+
<Tooltip
397+
id="sqllab-rowcount-tooltip"
398+
title={tooltipText}
399+
placement="left"
368400
>
369-
{limitMessage && (
370-
<Icons.ExclamationCircleOutlined
371-
css={css`
372-
font-size: ${theme.typography.sizes.m}px;
373-
margin-right: ${theme.gridUnit}px;
374-
`}
375-
/>
376-
)}
377-
{tn('%s row', '%s rows', rows, formattedRowCount)}
378-
</Label>
379-
</Tooltip>
380-
)}
381-
{!limitReached && shouldUseDefaultDropdownAlert && (
382-
<div ref={calculateAlertRefHeight}>
383-
<Alert
384-
type="warning"
385-
message={t('%(rows)d rows returned', { rows })}
386-
onClose={() => setAlertIsOpen(false)}
387-
description={t(
388-
'The number of rows displayed is limited to %(rows)d by the dropdown.',
389-
{ rows },
390-
)}
391-
/>
392-
</div>
393-
)}
394-
{limitReached && (
395-
<div ref={calculateAlertRefHeight}>
396-
<Alert
397-
type="warning"
398-
onClose={() => setAlertIsOpen(false)}
399-
message={t('%(rows)d rows returned', { rows: rowsCount })}
400-
description={
401-
isAdmin
402-
? displayMaxRowsReachedMessage.withAdmin
403-
: displayMaxRowsReachedMessage.withoutAdmin
404-
}
405-
/>
406-
</div>
401+
<Label
402+
css={css`
403+
line-height: ${theme.typography.sizes.l}px;
404+
`}
405+
>
406+
{limitMessage && (
407+
<Icons.ExclamationCircleOutlined
408+
css={css`
409+
font-size: ${theme.typography.sizes.m}px;
410+
margin-right: ${theme.gridUnit}px;
411+
`}
412+
/>
413+
)}
414+
{tn('%s row', '%s rows', rows, formattedRowCount)}
415+
</Label>
416+
</Tooltip>
417+
</ReturnedRows>
407418
)}
408-
</ReturnedRows>
419+
</>
409420
);
410421
};
411422

@@ -531,35 +542,39 @@ const ResultSet = ({
531542
<ResultContainer>
532543
{renderControls()}
533544
{showSql && showSqlInline ? (
534-
<div
535-
css={css`
536-
display: flex;
537-
justify-content: space-between;
538-
gap: ${GAP}px;
539-
`}
540-
>
541-
<Card
542-
css={[
543-
css`
544-
height: 28px;
545-
width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
546-
code {
547-
width: 100%;
548-
overflow: hidden;
549-
white-space: nowrap !important;
550-
text-overflow: ellipsis;
551-
display: block;
552-
}
553-
`,
554-
]}
545+
<>
546+
<div
547+
css={css`
548+
display: flex;
549+
justify-content: space-between;
550+
gap: ${GAP}px;
551+
`}
555552
>
556-
{sql}
557-
</Card>
558-
{renderRowsReturned()}
559-
</div>
553+
<Card
554+
css={[
555+
css`
556+
height: 28px;
557+
width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
558+
code {
559+
width: 100%;
560+
overflow: hidden;
561+
white-space: nowrap !important;
562+
text-overflow: ellipsis;
563+
display: block;
564+
}
565+
`,
566+
]}
567+
>
568+
{sql}
569+
</Card>
570+
{renderRowsReturned(false)}
571+
</div>
572+
{renderRowsReturned(true)}
573+
</>
560574
) : (
561575
<>
562-
{renderRowsReturned()}
576+
{renderRowsReturned(false)}
577+
{renderRowsReturned(true)}
563578
{sql}
564579
</>
565580
)}

superset-frontend/src/SqlLab/reducers/sqlLab.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,11 @@ export default function sqlLabReducer(state = {}, action) {
637637
// when it started fetching or finished rendering results
638638
state:
639639
currentState === QueryState.SUCCESS &&
640-
[QueryState.FETCHING, QueryState.SUCCESS].includes(prevState)
640+
[
641+
QueryState.FETCHING,
642+
QueryState.SUCCESS,
643+
QueryState.RUNNING,
644+
].includes(prevState)
641645
? prevState
642646
: currentState,
643647
};

0 commit comments

Comments
 (0)