Skip to content

Commit

Permalink
fix(sqllab): misplaced limit warning alert (apache#25306)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored Sep 27, 2023
1 parent 1716b9f commit 463962a
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 78 deletions.
169 changes: 92 additions & 77 deletions superset-frontend/src/SqlLab/components/ResultSet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ const ResultSet = ({
return <div />;
};

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

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

if (alertMessage) {
return (
<>
{!limitReached && shouldUseDefaultDropdownAlert && (
<div ref={calculateAlertRefHeight}>
<Alert
type="warning"
message={t('%(rows)d rows returned', { rows })}
onClose={() => setAlertIsOpen(false)}
description={t(
'The number of rows displayed is limited to %(rows)d by the dropdown.',
{ rows },
)}
/>
</div>
)}
{limitReached && (
<div ref={calculateAlertRefHeight}>
<Alert
type="warning"
onClose={() => setAlertIsOpen(false)}
message={t('%(rows)d rows returned', { rows: rowsCount })}
description={
isAdmin
? displayMaxRowsReachedMessage.withAdmin
: displayMaxRowsReachedMessage.withoutAdmin
}
/>
</div>
)}
</>
);
}
const showRowsReturned =
showSqlInline || (!limitReached && !shouldUseDefaultDropdownAlert);

return (
<ReturnedRows>
{!limitReached && !shouldUseDefaultDropdownAlert && (
<Tooltip
id="sqllab-rowcount-tooltip"
title={tooltipText}
placement="left"
>
<Label
css={css`
line-height: ${theme.typography.sizes.l}px;
`}
<>
{showRowsReturned && (
<ReturnedRows>
<Tooltip
id="sqllab-rowcount-tooltip"
title={tooltipText}
placement="left"
>
{limitMessage && (
<Icons.ExclamationCircleOutlined
css={css`
font-size: ${theme.typography.sizes.m}px;
margin-right: ${theme.gridUnit}px;
`}
/>
)}
{tn('%s row', '%s rows', rows, formattedRowCount)}
</Label>
</Tooltip>
)}
{!limitReached && shouldUseDefaultDropdownAlert && (
<div ref={calculateAlertRefHeight}>
<Alert
type="warning"
message={t('%(rows)d rows returned', { rows })}
onClose={() => setAlertIsOpen(false)}
description={t(
'The number of rows displayed is limited to %(rows)d by the dropdown.',
{ rows },
)}
/>
</div>
)}
{limitReached && (
<div ref={calculateAlertRefHeight}>
<Alert
type="warning"
onClose={() => setAlertIsOpen(false)}
message={t('%(rows)d rows returned', { rows: rowsCount })}
description={
isAdmin
? displayMaxRowsReachedMessage.withAdmin
: displayMaxRowsReachedMessage.withoutAdmin
}
/>
</div>
<Label
css={css`
line-height: ${theme.typography.sizes.l}px;
`}
>
{limitMessage && (
<Icons.ExclamationCircleOutlined
css={css`
font-size: ${theme.typography.sizes.m}px;
margin-right: ${theme.gridUnit}px;
`}
/>
)}
{tn('%s row', '%s rows', rows, formattedRowCount)}
</Label>
</Tooltip>
</ReturnedRows>
)}
</ReturnedRows>
</>
);
};

Expand Down Expand Up @@ -531,35 +542,39 @@ const ResultSet = ({
<ResultContainer>
{renderControls()}
{showSql && showSqlInline ? (
<div
css={css`
display: flex;
justify-content: space-between;
gap: ${GAP}px;
`}
>
<Card
css={[
css`
height: 28px;
width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
code {
width: 100%;
overflow: hidden;
white-space: nowrap !important;
text-overflow: ellipsis;
display: block;
}
`,
]}
<>
<div
css={css`
display: flex;
justify-content: space-between;
gap: ${GAP}px;
`}
>
{sql}
</Card>
{renderRowsReturned()}
</div>
<Card
css={[
css`
height: 28px;
width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
code {
width: 100%;
overflow: hidden;
white-space: nowrap !important;
text-overflow: ellipsis;
display: block;
}
`,
]}
>
{sql}
</Card>
{renderRowsReturned(false)}
</div>
{renderRowsReturned(true)}
</>
) : (
<>
{renderRowsReturned()}
{renderRowsReturned(false)}
{renderRowsReturned(true)}
{sql}
</>
)}
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/SqlLab/reducers/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,11 @@ export default function sqlLabReducer(state = {}, action) {
// when it started fetching or finished rendering results
state:
currentState === QueryState.SUCCESS &&
[QueryState.FETCHING, QueryState.SUCCESS].includes(prevState)
[
QueryState.FETCHING,
QueryState.SUCCESS,
QueryState.RUNNING,
].includes(prevState)
? prevState
: currentState,
};
Expand Down

0 comments on commit 463962a

Please sign in to comment.