From bb4b1ddc24ac03dbbdf53252806a48e0724cc034 Mon Sep 17 00:00:00 2001 From: Eric Briscoe Date: Wed, 1 Jun 2022 19:01:14 -0700 Subject: [PATCH] Fixes issue where results panel height was incorrect [sc-49045] (#20204) This commit fixes a dynamic height assignment issue where the SQL Editor results panel would be clipped offscreen and user could not see bottom of results, the height got assigned to zero after toggling online, then offline, and height would be calculated wrong if the result set rows returned message above the results table was long enough for a line wrap. --- .../src/SqlLab/components/ResultSet/index.tsx | 75 +++++++++++-------- .../src/SqlLab/components/SouthPane/index.tsx | 10 ++- 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index f4b39b93a279f..14931220ab17a 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -127,11 +127,8 @@ const MonospaceDiv = styled.div` const ReturnedRows = styled.div` font-size: 13px; line-height: 24px; - .limitMessage { - color: ${({ theme }) => theme.colors.secondary.light1}; - margin-left: ${({ theme }) => theme.gridUnit * 2}px; - } `; + const ResultSetControls = styled.div` display: flex; justify-content: space-between; @@ -148,6 +145,19 @@ const ResultSetErrorMessage = styled.div` padding-top: ${({ theme }) => 4 * theme.gridUnit}px; `; +const ResultSetRowsReturned = styled.span` + white-space: nowrap; + text-overflow: ellipsis; + width: 100%; + overflow: hidden; + display: inline-block; +`; + +const LimitMessage = styled.span` + color: ${({ theme }) => theme.colors.secondary.light1}; + margin-left: ${({ theme }) => theme.gridUnit * 2}px; +`; + const updateDataset = async ( dbId: number, datasetId: number, @@ -609,42 +619,38 @@ export default class ResultSet extends React.PureComponent< limitingFactor === LIMITING_FACTOR.DROPDOWN; if (limitingFactor === LIMITING_FACTOR.QUERY && this.props.csv) { - limitMessage = ( - - {t( - 'The number of rows displayed is limited to %(rows)d by the query', - { rows }, - )} - + limitMessage = t( + 'The number of rows displayed is limited to %(rows)d by the query', + { rows }, ); } else if ( limitingFactor === LIMITING_FACTOR.DROPDOWN && !shouldUseDefaultDropdownAlert ) { - limitMessage = ( - - {t( - 'The number of rows displayed is limited to %(rows)d by the limit dropdown.', - { rows }, - )} - + limitMessage = t( + 'The number of rows displayed is limited to %(rows)d by the limit dropdown.', + { rows }, ); } else if (limitingFactor === LIMITING_FACTOR.QUERY_AND_DROPDOWN) { - limitMessage = ( - - {t( - 'The number of rows displayed is limited to %(rows)d by the query and limit dropdown.', - { rows }, - )} - + limitMessage = t( + 'The number of rows displayed is limited to %(rows)d by the query and limit dropdown.', + { rows }, ); } + + const rowsReturnedMessage = t('%(rows)d rows returned', { + rows, + }); + + const tooltipText = `${rowsReturnedMessage}. ${limitMessage}`; + return ( {!limitReached && !shouldUseDefaultDropdownAlert && ( - - {t('%(rows)d rows returned', { rows })} {limitMessage} - + + {rowsReturnedMessage} + {limitMessage} + )} {!limitReached && shouldUseDefaultDropdownAlert && (
@@ -679,6 +685,7 @@ export default class ResultSet extends React.PureComponent< render() { const { query } = this.props; + const limitReached = query?.results?.displayLimitReached; let sql; let exploreDBId = query.dbId; if (this.props.database && this.props.database.explore_database_id) { @@ -748,9 +755,17 @@ export default class ResultSet extends React.PureComponent< } if (query.state === 'success' && query.results) { const { results } = query; + // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached + const rowMessageHeight = !limitReached ? 32 : 0; + // Accounts for offset needed for height of Alert if this.state.alertIsOpen + const alertContainerHeight = 70; + // We need to calculate the height of this.renderRowsReturned() + // if we want results panel to be propper height because the + // FilterTable component nedds an explcit height to render + // react-virtualized Table component const height = this.state.alertIsOpen - ? this.props.height - 70 - : this.props.height; + ? this.props.height - alertContainerHeight + : this.props.height - rowMessageHeight; let data; if (this.props.cache && query.cached) { ({ data } = this.state); diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index 12e6f8890c0da..db4bfa92f5c62 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -62,9 +62,13 @@ interface SouthPanePropTypes { defaultQueryLimit: number; } -const StyledPane = styled.div` - width: 100%; +type StyledPaneProps = { + height: number; +}; +const StyledPane = styled.div` + width: 100%; + height: ${props => props.height}px; .ant-tabs .ant-tabs-content-holder { overflow: visible; } @@ -207,7 +211,7 @@ export default function SouthPane({ return offline ? ( renderOfflineStatus() ) : ( - +