Skip to content

Commit

Permalink
Fixes issue where results panel height was incorrect [sc-49045] (apac…
Browse files Browse the repository at this point in the history
…he#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.
  • Loading branch information
eric-briscoe authored and philipher29 committed Jun 9, 2022
1 parent e9e724d commit bb4b1dd
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 33 deletions.
75 changes: 45 additions & 30 deletions superset-frontend/src/SqlLab/components/ResultSet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -609,42 +619,38 @@ export default class ResultSet extends React.PureComponent<
limitingFactor === LIMITING_FACTOR.DROPDOWN;

if (limitingFactor === LIMITING_FACTOR.QUERY && this.props.csv) {
limitMessage = (
<span className="limitMessage">
{t(
'The number of rows displayed is limited to %(rows)d by the query',
{ rows },
)}
</span>
limitMessage = t(
'The number of rows displayed is limited to %(rows)d by the query',
{ rows },
);
} else if (
limitingFactor === LIMITING_FACTOR.DROPDOWN &&
!shouldUseDefaultDropdownAlert
) {
limitMessage = (
<span className="limitMessage">
{t(
'The number of rows displayed is limited to %(rows)d by the limit dropdown.',
{ rows },
)}
</span>
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 = (
<span className="limitMessage">
{t(
'The number of rows displayed is limited to %(rows)d by the query and limit dropdown.',
{ rows },
)}
</span>
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 (
<ReturnedRows>
{!limitReached && !shouldUseDefaultDropdownAlert && (
<span>
{t('%(rows)d rows returned', { rows })} {limitMessage}
</span>
<ResultSetRowsReturned title={tooltipText}>
{rowsReturnedMessage}
<LimitMessage>{limitMessage}</LimitMessage>
</ResultSetRowsReturned>
)}
{!limitReached && shouldUseDefaultDropdownAlert && (
<div ref={this.calculateAlertRefHeight}>
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 7 additions & 3 deletions superset-frontend/src/SqlLab/components/SouthPane/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,13 @@ interface SouthPanePropTypes {
defaultQueryLimit: number;
}

const StyledPane = styled.div`
width: 100%;
type StyledPaneProps = {
height: number;
};

const StyledPane = styled.div<StyledPaneProps>`
width: 100%;
height: ${props => props.height}px;
.ant-tabs .ant-tabs-content-holder {
overflow: visible;
}
Expand Down Expand Up @@ -207,7 +211,7 @@ export default function SouthPane({
return offline ? (
renderOfflineStatus()
) : (
<StyledPane className="SouthPane" ref={southPaneRef}>
<StyledPane className="SouthPane" height={height} ref={southPaneRef}>
<Tabs
activeKey={activeSouthPaneTab}
className="SouthPaneTabs"
Expand Down

0 comments on commit bb4b1dd

Please sign in to comment.