From a4bd8938d9670be1cf5bceb04d61338cc9bfbbda Mon Sep 17 00:00:00 2001 From: Talyor Chen Date: Tue, 1 Mar 2022 13:28:28 -0500 Subject: [PATCH 1/7] fix(column-header-tooltip): make that hide the tooltip when the cloumn header is turncated --- .../components/FilterableTable/FilterableTable.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index 23d68f96850c9..1bcf0ce1fb785 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -29,7 +29,7 @@ import { SortIndicator, Table, } from 'react-virtualized'; -import { getMultipleTextDimensions, t, styled } from '@superset-ui/core'; +import { getTextDimension, getMultipleTextDimensions, t, styled } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import Button from '../Button'; import CopyToClipboard from '../CopyToClipboard'; @@ -95,6 +95,7 @@ const StyledFilterableTable = styled.div` // when more than MAX_COLUMNS_FOR_TABLE are returned, switch from table to grid view export const MAX_COLUMNS_FOR_TABLE = 50; +export const MAX_COLUMN_WIDTH = 200; type CellDataType = string | number | null; type Datum = Record; @@ -213,13 +214,14 @@ export default class FilterableTable extends PureComponent< // we can't use Math.max(...colWidths.slice(...)) here since the number // of elements might be bigger than the number of allowed arguments in a // Javascript function - widthsByColumnKey[key] = + const value = colWidths .slice( index * (this.list.length + 1), (index + 1) * (this.list.length + 1), ) .reduce((a, b) => Math.max(a, b)) + PADDING; + widthsByColumnKey[key] = value > MAX_COLUMN_WIDTH ? MAX_COLUMN_WIDTH : value; }); return widthsByColumnKey; @@ -413,10 +415,16 @@ export default class FilterableTable extends PureComponent< this.props.expandedColumns.indexOf(label) > -1 ? 'header-style-disabled' : 'header-style'; + + let trigger: string[] = []; + if (this.widthsForColumnsByKey[dataKey] < getTextDimension({ style: {fontSize: '12px'}, text: label}).width) { + trigger=['hover']; + } return ( From 61e16a0de91739785b293254fff88f8c5c4bb0a8 Mon Sep 17 00:00:00 2001 From: Talyor Chen Date: Tue, 1 Mar 2022 13:45:10 -0500 Subject: [PATCH 2/7] fix(column-header-tooltip): fix lint --- .../FilterableTable/FilterableTable.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index 1bcf0ce1fb785..aa5d7759002f8 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -29,7 +29,12 @@ import { SortIndicator, Table, } from 'react-virtualized'; -import { getTextDimension, getMultipleTextDimensions, t, styled } from '@superset-ui/core'; +import { + getTextDimension, + getMultipleTextDimensions, + t, + styled, +} from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import Button from '../Button'; import CopyToClipboard from '../CopyToClipboard'; @@ -221,7 +226,8 @@ export default class FilterableTable extends PureComponent< (index + 1) * (this.list.length + 1), ) .reduce((a, b) => Math.max(a, b)) + PADDING; - widthsByColumnKey[key] = value > MAX_COLUMN_WIDTH ? MAX_COLUMN_WIDTH : value; + widthsByColumnKey[key] = + value > MAX_COLUMN_WIDTH ? MAX_COLUMN_WIDTH : value; }); return widthsByColumnKey; @@ -417,8 +423,11 @@ export default class FilterableTable extends PureComponent< : 'header-style'; let trigger: string[] = []; - if (this.widthsForColumnsByKey[dataKey] < getTextDimension({ style: {fontSize: '12px'}, text: label}).width) { - trigger=['hover']; + if ( + this.widthsForColumnsByKey[dataKey] < + getTextDimension({ style: { fontSize: '12px' }, text: label }).width + ) { + trigger = ['hover']; } return ( Date: Mon, 18 Apr 2022 15:25:19 -0400 Subject: [PATCH 3/7] fix(column-header-tooltip): make to dynamic tooltip header in FilterTable --- .../FilterableTable/FilterableTable.tsx | 54 +++++++++++-------- .../src/components/TooltipParagraph/index.tsx | 43 +++++++++++++++ 2 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 superset-frontend/src/components/TooltipParagraph/index.tsx diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index aa5d7759002f8..20d743dd11211 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -30,12 +30,12 @@ import { Table, } from 'react-virtualized'; import { - getTextDimension, getMultipleTextDimensions, t, styled, } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; +import TooltipParagraph from 'src/components/TooltipParagraph'; import Button from '../Button'; import CopyToClipboard from '../CopyToClipboard'; import ModalTrigger from '../ModalTrigger'; @@ -100,7 +100,6 @@ const StyledFilterableTable = styled.div` // when more than MAX_COLUMNS_FOR_TABLE are returned, switch from table to grid view export const MAX_COLUMNS_FOR_TABLE = 50; -export const MAX_COLUMN_WIDTH = 200; type CellDataType = string | number | null; type Datum = Record; @@ -226,8 +225,7 @@ export default class FilterableTable extends PureComponent< (index + 1) * (this.list.length + 1), ) .reduce((a, b) => Math.max(a, b)) + PADDING; - widthsByColumnKey[key] = - value > MAX_COLUMN_WIDTH ? MAX_COLUMN_WIDTH : value; + widthsByColumnKey[key] = value; }); return widthsByColumnKey; @@ -422,18 +420,10 @@ export default class FilterableTable extends PureComponent< ? 'header-style-disabled' : 'header-style'; - let trigger: string[] = []; - if ( - this.widthsForColumnsByKey[dataKey] < - getTextDimension({ style: { fontSize: '12px' }, text: label }).width - ) { - trigger = ['hover']; - } return ( @@ -462,13 +452,6 @@ export default class FilterableTable extends PureComponent< ? 'header-style-disabled' : 'header-style'; return ( -
this.sortGrid(label)} > - {label} + + {label} + {this.state.sortBy === label && ( )}
-
+ // + //
this.sortGrid(label)} + // > + // {label} + // {this.state.sortBy === label && ( + // + // )} + //
+ //
); } diff --git a/superset-frontend/src/components/TooltipParagraph/index.tsx b/superset-frontend/src/components/TooltipParagraph/index.tsx new file mode 100644 index 0000000000000..4c96082b39ec5 --- /dev/null +++ b/superset-frontend/src/components/TooltipParagraph/index.tsx @@ -0,0 +1,43 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { useState } from "react"; +import { Tooltip } from "antd"; +import Paragraph, { ParagraphProps } from "antd/es/typography/Paragraph"; + +const TooltipParagraph: React.FC = ({ + children, + ellipsis, + ...props +}) => { + const [truncated, setTruncated] = useState(false); + + return ( + + + {/* NOTE: Fragment is necessary to avoid showing the title */} + <>{children} + + + ); +}; + +export default TooltipParagraph; From 31be89c59993ce5324880116d4fe0ef3c2bdc246 Mon Sep 17 00:00:00 2001 From: Taylor Date: Mon, 18 Apr 2022 15:59:30 -0400 Subject: [PATCH 4/7] fix(column-header-tooltip): make to fix the lint issue --- .../FilterableTable/FilterableTable.tsx | 48 ++++++++----------- .../src/components/TooltipParagraph/index.tsx | 6 +-- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index 20d743dd11211..893cd254434bf 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -29,11 +29,7 @@ import { SortIndicator, Table, } from 'react-virtualized'; -import { - getMultipleTextDimensions, - t, - styled, -} from '@superset-ui/core'; +import { getMultipleTextDimensions, t, styled } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import TooltipParagraph from 'src/components/TooltipParagraph'; import Button from '../Button'; @@ -452,28 +448,26 @@ export default class FilterableTable extends PureComponent< ? 'header-style-disabled' : 'header-style'; return ( -
this.sortGrid(label)} - > - - {label} - - {this.state.sortBy === label && ( - - )} -
+
this.sortGrid(label)} + > + + {label} + + {this.state.sortBy === label && ( + + )} +
// = ({ children, From 763dd7aacf955f2b5d2efc6c88c2dabbfaf0514c Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 19 Apr 2022 09:47:41 -0400 Subject: [PATCH 5/7] fix(column-header-tooltip): make to remove the tooltip option --- .../FilterableTable/FilterableTable.tsx | 50 +++---------------- 1 file changed, 6 insertions(+), 44 deletions(-) diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index 893cd254434bf..50b7ecd0d7609 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -30,8 +30,6 @@ import { Table, } from 'react-virtualized'; import { getMultipleTextDimensions, t, styled } from '@superset-ui/core'; -import { Tooltip } from 'src/components/Tooltip'; -import TooltipParagraph from 'src/components/TooltipParagraph'; import Button from '../Button'; import CopyToClipboard from '../CopyToClipboard'; import ModalTrigger from '../ModalTrigger'; @@ -417,19 +415,10 @@ export default class FilterableTable extends PureComponent< : 'header-style'; return ( - -
- {label} - {sortBy === dataKey && ( - - )} -
-
+
+ {label} + {sortBy === dataKey && } +
); } @@ -449,6 +438,7 @@ export default class FilterableTable extends PureComponent< : 'header-style'; return (
this.sortGrid(label)} > - - {label} - + {label} {this.state.sortBy === label && ( )}
- // - //
this.sortGrid(label)} - // > - // {label} - // {this.state.sortBy === label && ( - // - // )} - //
- //
); } From c830b29dc253d217da34ba18a81673b590f3e61d Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 19 Apr 2022 14:40:43 -0400 Subject: [PATCH 6/7] fix(column-header-tooltip): make to add test and storybook for dynamic tooltip --- .../TooltipParagraph.stories.tsx | 52 ++++++++++++++++ .../TooltipParagraph.test.tsx | 60 +++++++++++++++++++ .../src/components/TooltipParagraph/index.tsx | 8 +-- 3 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx create mode 100644 superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx diff --git a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx new file mode 100644 index 0000000000000..b0507162cd3b9 --- /dev/null +++ b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx @@ -0,0 +1,52 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + import React from 'react'; + import TooltipParagraph from './'; + + export default { + title: 'DynamicTooltip', + component: TooltipParagraph, + }; + + type IProps = { + title: string; + width: number; + } + + export const InteractiveTooltip = (args: IProps) => ( +
+ + {args.title} + +
+ ); + + InteractiveTooltip.story = { + parameters: { + knobs: { + disable: true, + }, + }, + }; + + InteractiveTooltip.args = { + title: 'This is too long and should truncate.', + width: 200, + }; + \ No newline at end of file diff --git a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx new file mode 100644 index 0000000000000..5c0cf47f4f576 --- /dev/null +++ b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx @@ -0,0 +1,60 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import TooltipParagraph from '.'; + +test('starts hidden with default props', () => { + render( + + This is tootlip description. + , + ); + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); +}); + +test('not render on hover when not truncated', async () => { + const {container} = render( +
+ + This is short. + +
+ ); + userEvent.hover(screen.getByRole('hoverText')); + await waitFor(() => + expect(container.firstChild?.firstChild).not.toHaveClass('ant-tooltip-open') + ); +}); + +test('render on hover when truncated', async () => { + const {container} = render( +
+ + This is too long and should truncate. + +
+ ); + userEvent.hover(screen.getByRole('hoverText')); + await waitFor(() => + expect(container.firstChild?.firstChild).toHaveClass('ant-tooltip-open') + ); +}); + \ No newline at end of file diff --git a/superset-frontend/src/components/TooltipParagraph/index.tsx b/superset-frontend/src/components/TooltipParagraph/index.tsx index d86d4f60b8ad5..35543643f8ffd 100644 --- a/superset-frontend/src/components/TooltipParagraph/index.tsx +++ b/superset-frontend/src/components/TooltipParagraph/index.tsx @@ -17,8 +17,8 @@ * under the License. */ import React, { useState } from 'react'; -import { Tooltip } from 'antd'; -import Paragraph, { ParagraphProps } from 'antd/es/typography/Paragraph'; +import { Tooltip, Typography } from 'antd'; +import { ParagraphProps } from 'antd/es/typography/Paragraph'; const TooltipParagraph: React.FC = ({ children, @@ -29,13 +29,13 @@ const TooltipParagraph: React.FC = ({ return ( - {/* NOTE: Fragment is necessary to avoid showing the title */} <>{children} - + ); }; From 16831011da29267fe30438f6ea8402dc73d7ca56 Mon Sep 17 00:00:00 2001 From: Taylor Date: Tue, 19 Apr 2022 15:20:03 -0400 Subject: [PATCH 7/7] fix(column-header-tooltip): make to fix lint --- .../TooltipParagraph.stories.tsx | 63 +++++++++---------- .../TooltipParagraph.test.tsx | 33 +++++----- 2 files changed, 45 insertions(+), 51 deletions(-) diff --git a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx index b0507162cd3b9..b50bbd8485de5 100644 --- a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx +++ b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.stories.tsx @@ -16,37 +16,34 @@ * specific language governing permissions and limitations * under the License. */ - import React from 'react'; - import TooltipParagraph from './'; - - export default { - title: 'DynamicTooltip', - component: TooltipParagraph, - }; +import React from 'react'; +import TooltipParagraph from '.'; - type IProps = { - title: string; - width: number; - } - - export const InteractiveTooltip = (args: IProps) => ( -
- - {args.title} - -
- ); - - InteractiveTooltip.story = { - parameters: { - knobs: { - disable: true, - }, - }, - }; - - InteractiveTooltip.args = { - title: 'This is too long and should truncate.', - width: 200, - }; - \ No newline at end of file +export default { + title: 'DynamicTooltip', + component: TooltipParagraph, +}; + +type IProps = { + title: string; + width: number; +}; + +export const InteractiveTooltip = (args: IProps) => ( +
+ {args.title} +
+); + +InteractiveTooltip.story = { + parameters: { + knobs: { + disable: true, + }, + }, +}; + +InteractiveTooltip.args = { + title: 'This is too long and should truncate.', + width: 200, +}; diff --git a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx index 5c0cf47f4f576..660df3489a9c2 100644 --- a/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx +++ b/superset-frontend/src/components/TooltipParagraph/TooltipParagraph.test.tsx @@ -22,39 +22,36 @@ import userEvent from '@testing-library/user-event'; import TooltipParagraph from '.'; test('starts hidden with default props', () => { - render( - - This is tootlip description. - , - ); + render(This is tootlip description.); expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); }); test('not render on hover when not truncated', async () => { - const {container} = render( -
+ const { container } = render( +
- This is short. + This is short -
+
, ); - userEvent.hover(screen.getByRole('hoverText')); + userEvent.hover(screen.getByTestId('test-text')); await waitFor(() => - expect(container.firstChild?.firstChild).not.toHaveClass('ant-tooltip-open') + expect(container.firstChild?.firstChild).not.toHaveClass( + 'ant-tooltip-open', + ), ); }); test('render on hover when truncated', async () => { - const {container} = render( -
+ const { container } = render( +
- This is too long and should truncate. + This is too long and should truncate. -
+
, ); - userEvent.hover(screen.getByRole('hoverText')); + userEvent.hover(screen.getByTestId('test-text')); await waitFor(() => - expect(container.firstChild?.firstChild).toHaveClass('ant-tooltip-open') + expect(container.firstChild?.firstChild).toHaveClass('ant-tooltip-open'), ); }); - \ No newline at end of file