Skip to content

Commit

Permalink
fix(dashboard): Fix BigNumber causing dashboard to crash when overflo…
Browse files Browse the repository at this point in the history
…wing (apache#19688)

* fix(dashboard): fix(plugin-chart-echarts): Fix BigNumber causing dashboard to crash when overflowing

* Add tooltips for truncated titles

* Fix type
  • Loading branch information
kgabryje committed Apr 13, 2022
1 parent 4a5dddf commit ee85466
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 22 deletions.
9 changes: 4 additions & 5 deletions superset-frontend/src/components/EditableTitle/index.tsx
Expand Up @@ -57,6 +57,8 @@ export default function EditableTitle({
placeholder = '',
certifiedBy,
certificationDetails,
// rest is related to title tooltip
...rest
}: EditableTitleProps) {
const [isEditing, setIsEditing] = useState(editing);
const [currentTitle, setCurrentTitle] = useState(title);
Expand Down Expand Up @@ -214,11 +216,7 @@ export default function EditableTitle({
}
if (!canEdit) {
// don't actually want an input in this case
titleComponent = (
<span data-test="editable-title-input" title={value}>
{value}
</span>
);
titleComponent = <span data-test="editable-title-input">{value}</span>;
}
return (
<span
Expand All @@ -230,6 +228,7 @@ export default function EditableTitle({
isEditing && 'editable-title--editing',
)}
style={style}
{...rest}
>
{certifiedBy && (
<>
Expand Down
Expand Up @@ -157,6 +157,8 @@ const createProps = () => ({
exportCSV: jest.fn(),
onExploreChart: jest.fn(),
formData: { slice_id: 1, datasource: '58__table' },
width: 100,
height: 100,
});

test('Should render', () => {
Expand Down
49 changes: 35 additions & 14 deletions superset-frontend/src/dashboard/components/SliceHeader/index.tsx
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { FC, useMemo } from 'react';
import React, { FC, useEffect, useMemo, useRef, useState } from 'react';
import { styled, t } from '@superset-ui/core';
import { useUiConfig } from 'src/components/UiConfigContext';
import { Tooltip } from 'src/components/Tooltip';
Expand All @@ -41,6 +41,8 @@ type SliceHeaderProps = SliceHeaderControlsProps & {
filters: object;
handleToggleFullSize: () => void;
formData: object;
width: number;
height: number;
};

const annotationsLoading = t('Annotation layers are still loading.');
Expand Down Expand Up @@ -82,9 +84,13 @@ const SliceHeader: FC<SliceHeaderProps> = ({
isFullSize,
chartStatus,
formData,
width,
height,
}) => {
const dispatch = useDispatch();
const uiConfig = useUiConfig();
const [headerTooltip, setHeaderTooltip] = useState<string | null>(null);
const headerRef = useRef<HTMLDivElement>(null);
// TODO: change to indicator field after it will be implemented
const crossFilterValue = useSelector<RootState, any>(
state => state.dataMask[slice?.slice_id]?.filterState?.value,
Expand All @@ -98,21 +104,36 @@ const SliceHeader: FC<SliceHeaderProps> = ({
[crossFilterValue],
);

useEffect(() => {
const headerElement = headerRef.current;
if (
headerElement &&
(headerElement.scrollWidth > headerElement.offsetWidth ||
headerElement.scrollHeight > headerElement.offsetHeight)
) {
setHeaderTooltip(sliceName ?? null);
} else {
setHeaderTooltip(null);
}
}, [sliceName, width, height]);

return (
<div className="chart-header" data-test="slice-header" ref={innerRef}>
<div className="header-title">
<EditableTitle
title={
sliceName ||
(editMode
? '---' // this makes an empty title clickable
: '')
}
canEdit={editMode}
emptyText=""
onSaveTitle={updateSliceName}
showTooltip={false}
/>
<div className="header-title" ref={headerRef}>
<Tooltip title={headerTooltip}>
<EditableTitle
title={
sliceName ||
(editMode
? '---' // this makes an empty title clickable
: '')
}
canEdit={editMode}
emptyText=""
onSaveTitle={updateSliceName}
showTooltip={false}
/>
</Tooltip>
{!!Object.values(annotationQuery).length && (
<Tooltip
id="annotations-loading-tooltip"
Expand Down
Expand Up @@ -113,6 +113,12 @@ const ChartOverlay = styled.div`
}
`;

const SliceContainer = styled.div`
display: flex;
flex-direction: column;
max-height: 100%;
`;

export default class Chart extends React.Component {
constructor(props) {
super(props);
Expand Down Expand Up @@ -210,7 +216,10 @@ export default class Chart extends React.Component {

getChartHeight() {
const headerHeight = this.getHeaderHeight();
return this.state.height - headerHeight - this.state.descriptionHeight;
return Math.max(
this.state.height - headerHeight - this.state.descriptionHeight,
20,
);
}

getHeaderHeight() {
Expand Down Expand Up @@ -370,7 +379,7 @@ export default class Chart extends React.Component {
})
: {};
return (
<div
<SliceContainer
className="chart-slice"
data-test="chart-grid-component"
data-test-chart-id={id}
Expand Down Expand Up @@ -407,6 +416,8 @@ export default class Chart extends React.Component {
isFullSize={isFullSize}
chartStatus={chart.chartStatus}
formData={formData}
width={width}
height={this.getHeaderHeight()}
/>

{/*
Expand Down Expand Up @@ -468,7 +479,7 @@ export default class Chart extends React.Component {
datasetsStatus={datasetsStatus}
/>
</div>
</div>
</SliceContainer>
);
}
}
Expand Down
8 changes: 8 additions & 0 deletions superset-frontend/src/dashboard/stylesheets/dashboard.less
Expand Up @@ -63,12 +63,20 @@ body {
display: flex;
max-width: 100%;
align-items: flex-start;
min-height: 0;

& > .header-title {
overflow: hidden;
text-overflow: ellipsis;
max-width: 100%;
flex-grow: 1;
display: -webkit-box;
-webkit-line-clamp: 2;
-webkit-box-orient: vertical;

& > span.ant-tooltip-open {
display: inline;
}
}

& > .header-controls {
Expand Down

0 comments on commit ee85466

Please sign in to comment.