From 29284e22b38f832b3ff7e66343baee8395df219d Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 6 Oct 2022 17:11:18 +0300 Subject: [PATCH 01/15] Add CrossLinks component and dashboards filter --- .../src/components/ListView/CrossLinks.tsx | 104 ++++++++++++++++++ .../src/views/CRUD/chart/ChartList.tsx | 74 +++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 superset-frontend/src/components/ListView/CrossLinks.tsx diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx new file mode 100644 index 0000000000000..34da77fb63e4b --- /dev/null +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -0,0 +1,104 @@ +/** + * 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 { styled, t } from '@superset-ui/core'; +import { Tooltip } from 'src/components/Tooltip'; +import { Link } from 'react-router-dom'; + +export type CrossLinkProps = { + title: string; + id: number; +}; + +interface CrossLinksProps { + crossLinks: Array; + maxLinks?: number; + linkPrefix?: string; +} + +const StyledCrossLinks = styled.div` + color: ${({ theme }) => theme.colors.primary.dark1}; + cursor: pointer; + + .ant-tooltip-open { + max-width: 100%; + } + + .truncated { + max-width: calc(100% - 20px); + white-space: nowrap; + display: inline-block; + overflow: hidden; + vertical-align: bottom; + text-overflow: ellipsis; + } +`; + +const StyledLinkedTooltip = styled.div` + .link { + color: ${({ theme }) => theme.colors.grayscale.light5}; + display: block; + text-decoration: underline; + } +`; + +export default function CrossLinks({ + crossLinks, + maxLinks = 50, + linkPrefix = '/superset/dashboard/', +}: CrossLinksProps) { + return ( + + {crossLinks.length > 1 ? ( + + {crossLinks.slice(0, maxLinks).map(link => ( + + {link.title} + + ))} + {crossLinks.length > maxLinks && ( + {t('Plus %s more', crossLinks.length - maxLinks)} + )} + + } + > + + {crossLinks.map(link => link.title).join(', ')} + {' '} + +{crossLinks.length} + + ) : ( + + {crossLinks[0] && ( + + {crossLinks[0].title} + + )} + + )} + + ); +} diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 8fbf37392f870..6376fa4bcbad0 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -49,6 +49,7 @@ import ListView, { ListViewProps, SelectOption, } from 'src/components/ListView'; +import CrossLinks from 'src/components/ListView/CrossLinks'; import Loading from 'src/components/Loading'; import { dangerouslyGetItemDoNotUse } from 'src/utils/localStorageHelpers'; import withToasts from 'src/components/MessageToasts/withToasts'; @@ -135,6 +136,47 @@ const createFetchDatasets = async ( }; }; +const createFetchDashboards = async ( + filterValue = '', + page: number, + pageSize: number, +) => { + // add filters if filterValue + const filters = filterValue + ? { filters: [{ col: 'dashboards', opr: 'rel_m_m', value: filterValue }] } + : {}; + const queryParams = rison.encode({ + columns: ['dashboard_title', 'id'], + keys: ['none'], + order_column: 'dashboard_title', + order_direction: 'asc', + page, + page_size: pageSize, + ...filters, + }); + const { json = {} } = await SupersetClient.get({ + endpoint: !filterValue + ? `/api/v1/dashboard/?q=${queryParams}` + : `/api/v1/chart/?q=${queryParams}`, + }); + const dashboards = json?.result?.map( + ({ + dashboard_title: dashboardTitle, + id, + }: { + dashboard_title: string; + id: number; + }) => ({ + label: dashboardTitle, + value: id, + }), + ); + return { + data: uniqBy(dashboards, 'value'), + totalCount: json?.count, + }; +}; + interface ChartListProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; @@ -145,6 +187,11 @@ interface ChartListProps { }; } +type ChartLinkedDashboard = { + id: number; + dashboard_title: string; +}; + const Actions = styled.div` color: ${({ theme }) => theme.colors.grayscale.base}; `; @@ -324,6 +371,24 @@ function ChartList(props: ChartListProps) { disableSortBy: true, size: 'xl', }, + { + Cell: ({ + row: { + original: { dashboards }, + }, + }: any) => ( + ({ + title: d.dashboard_title, + id: d.id, + }))} + /> + ), + Header: t('Dashboards added to'), + accessor: 'dashboards', + disableSortBy: true, + size: 'xxl', + }, { Cell: ({ row: { @@ -568,6 +633,15 @@ function ChartList(props: ChartListProps) { fetchSelects: createFetchDatasets, paginate: true, }, + { + Header: t('Dashboards'), + id: 'dashboards', + input: 'select', + operator: FilterOperator.relationManyMany, + unfilteredLabel: t('All'), + fetchSelects: createFetchDashboards, + paginate: true, + }, ...(userId ? [favoritesFilter] : []), { Header: t('Certified'), From 3dfa0de32f373fe52273ba8c522e211eea795cdb Mon Sep 17 00:00:00 2001 From: geido Date: Fri, 7 Oct 2022 15:01:05 +0300 Subject: [PATCH 02/15] Use useTruncation --- .../src/components/ListView/CrossLinks.tsx | 69 ++++++++++++------- .../FilterCard/DependenciesRow.tsx | 2 +- .../nativeFilters/FilterCard/NameRow.tsx | 2 +- .../nativeFilters/FilterCard/ScopeRow.tsx | 2 +- .../useTruncation/index.ts} | 2 + 5 files changed, 49 insertions(+), 28 deletions(-) rename superset-frontend/src/{dashboard/components/nativeFilters/FilterCard/useTruncation.ts => hooks/useTruncation/index.ts} (99%) diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index 34da77fb63e4b..99bc83cb57c54 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -16,10 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useRef } from 'react'; import { styled, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import { Link } from 'react-router-dom'; +import { useTruncation } from 'src/hooks/useTruncation'; export type CrossLinkProps = { title: string; @@ -33,21 +34,29 @@ interface CrossLinksProps { } const StyledCrossLinks = styled.div` - color: ${({ theme }) => theme.colors.primary.dark1}; - cursor: pointer; + ${({ theme }) => ` + color: ${theme.colors.primary.dark1}; + cursor: pointer; + max-width: calc(100% - 20px); - .ant-tooltip-open { - max-width: 100%; - } + .ant-tooltip-open { + width: 100%; + } - .truncated { - max-width: calc(100% - 20px); - white-space: nowrap; - display: inline-block; - overflow: hidden; - vertical-align: bottom; - text-overflow: ellipsis; - } + .truncated { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + display: inline-block; + width: 100%; + vertical-align: bottom; + } + + .count { + color: ${theme.colors.grayscale.base}; + font-weight: ${theme.typography.weights.bold}; + } + `} `; const StyledLinkedTooltip = styled.div` @@ -63,6 +72,9 @@ export default function CrossLinks({ maxLinks = 50, linkPrefix = '/superset/dashboard/', }: CrossLinksProps) { + const crossLinksRef = useRef(null); + const [elementsTruncated, hasHiddenElements] = useTruncation(crossLinksRef); + return ( {crossLinks.length > 1 ? ( @@ -75,6 +87,7 @@ export default function CrossLinks({ key={link.id} to={linkPrefix + link.id} target="_blank" + rel="noreferer noopener" > {link.title} @@ -85,19 +98,25 @@ export default function CrossLinks({ } > - - {crossLinks.map(link => link.title).join(', ')} - {' '} - +{crossLinks.length} + + {crossLinks.map((element, index) => ( + {index === 0 ? element.title : `, ${element.title}`} + ))} + + {hasHiddenElements && ( + +{elementsTruncated} + )} ) : ( - - {crossLinks[0] && ( - - {crossLinks[0].title} - - )} - + crossLinks[0] && ( + + {crossLinks[0].title} + + ) )} ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx index 18a1c257b4ba3..abbaa1e63357c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx @@ -21,6 +21,7 @@ import { useDispatch } from 'react-redux'; import { css, t, useTheme } from '@superset-ui/core'; import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState'; import Icons from 'src/components/Icons'; +import { useTruncation } from 'src/hooks/useTruncation'; import { DependencyItem, Row, @@ -30,7 +31,6 @@ import { TooltipList, } from './Styles'; import { useFilterDependencies } from './useFilterDependencies'; -import { useTruncation } from './useTruncation'; import { DependencyValueProps, FilterCardRowProps } from './types'; import { TooltipWithTruncation } from './TooltipWithTruncation'; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx index 05cb8119487cd..f6268296efdc6 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/NameRow.tsx @@ -19,9 +19,9 @@ import React, { useRef } from 'react'; import { css, SupersetTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; +import { useTruncation } from 'src/hooks/useTruncation'; import { Row, FilterName } from './Styles'; import { FilterCardRowProps } from './types'; -import { useTruncation } from './useTruncation'; import { TooltipWithTruncation } from './TooltipWithTruncation'; export const NameRow = ({ filter }: FilterCardRowProps) => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx index 66656f0ba514d..6a672ed42c90e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx @@ -18,6 +18,7 @@ */ import React, { useMemo, useRef } from 'react'; import { t } from '@superset-ui/core'; +import { useTruncation } from 'src/hooks/useTruncation'; import { useFilterScope } from './useFilterScope'; import { Row, @@ -27,7 +28,6 @@ import { TooltipList, TooltipSectionLabel, } from './Styles'; -import { useTruncation } from './useTruncation'; import { FilterCardRowProps } from './types'; import { TooltipWithTruncation } from './TooltipWithTruncation'; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useTruncation.ts b/superset-frontend/src/hooks/useTruncation/index.ts similarity index 99% rename from superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useTruncation.ts rename to superset-frontend/src/hooks/useTruncation/index.ts index a4a893463f616..d000a6277afd9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useTruncation.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -61,12 +61,14 @@ export const useTruncation = (elementRef: RefObject) => { // "..." is around 6px wide const maxWidth = clientWidth - 6; const elementsCount = childNodes.length; + let width = 0; let i = 0; while (width < maxWidth) { width += (childNodes[i] as HTMLElement).offsetWidth; i += 1; } + console.log('i', i); if (i === elementsCount) { setElementsTruncated(1); setHasHiddenElements(false); From 723011d295af4c2b91bcd5f33af4d91659a90b96 Mon Sep 17 00:00:00 2001 From: geido Date: Fri, 7 Oct 2022 18:02:52 +0300 Subject: [PATCH 03/15] Add tests --- .../components/ListView/CrossLinks.test.tsx | 97 ++++++++++++++++++ .../src/components/ListView/CrossLinks.tsx | 98 ++++++++----------- .../ListView/CrossLinksTooltip.test.tsx | 88 +++++++++++++++++ .../components/ListView/CrossLinksTooltip.tsx | 69 +++++++++++++ .../src/hooks/useTruncation/index.ts | 1 - .../src/views/CRUD/chart/ChartList.tsx | 11 ++- 6 files changed, 303 insertions(+), 61 deletions(-) create mode 100644 superset-frontend/src/components/ListView/CrossLinks.test.tsx create mode 100644 superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx create mode 100644 superset-frontend/src/components/ListView/CrossLinksTooltip.tsx diff --git a/superset-frontend/src/components/ListView/CrossLinks.test.tsx b/superset-frontend/src/components/ListView/CrossLinks.test.tsx new file mode 100644 index 0000000000000..e50b32fe35c9a --- /dev/null +++ b/superset-frontend/src/components/ListView/CrossLinks.test.tsx @@ -0,0 +1,97 @@ +/** + * 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 } from 'spec/helpers/testing-library'; +import CrossLinks, { CrossLinksProps } from './CrossLinks'; + +const mockedProps = { + crossLinks: [ + { + id: 1, + title: 'Test dashboard', + }, + { + id: 2, + title: 'Test dashboard 2', + }, + { + id: 3, + title: 'Test dashboard 3', + }, + { + id: 4, + title: 'Test dashboard 4', + }, + ], +}; + +function setup(overrideProps: CrossLinksProps | {} = {}) { + return render(, { + useRouter: true, + }); +} + +test('should render', () => { + const { container } = setup(); + expect(container).toBeInTheDocument(); +}); + +test('should not render links', () => { + setup({ + crossLinks: [], + }); + expect(screen.queryByRole('link')).not.toBeInTheDocument(); +}); + +test('should render the link with just one item', () => { + setup({ + crossLinks: [ + { + id: 1, + title: 'Test dashboard', + }, + ], + }); + expect(screen.getByText('Test dashboard')).toBeInTheDocument(); + expect(screen.getByRole('link')).toHaveAttribute( + 'href', + `/superset/dashboard/1`, + ); +}); + +test('should render a custom prefix link', () => { + setup({ + crossLinks: [ + { + id: 1, + title: 'Test dashboard', + }, + ], + linkPrefix: '/preset/dashboard/', + }); + expect(screen.getByRole('link')).toHaveAttribute( + 'href', + `/preset/dashboard/1`, + ); +}); + +test('should render multiple links', () => { + setup(); + expect(screen.getAllByRole('link')).toHaveLength(4); +}); diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index 99bc83cb57c54..aa1796a6c784d 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -16,31 +16,31 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useRef } from 'react'; -import { styled, t } from '@superset-ui/core'; -import { Tooltip } from 'src/components/Tooltip'; +import React, { useMemo, useRef } from 'react'; +import { styled } from '@superset-ui/core'; import { Link } from 'react-router-dom'; import { useTruncation } from 'src/hooks/useTruncation'; +import CrossLinksTooltip from './CrossLinksTooltip'; export type CrossLinkProps = { title: string; id: number; }; -interface CrossLinksProps { +export type CrossLinksProps = { crossLinks: Array; maxLinks?: number; linkPrefix?: string; -} +}; const StyledCrossLinks = styled.div` ${({ theme }) => ` - color: ${theme.colors.primary.dark1}; cursor: pointer; - max-width: calc(100% - 20px); + position: relative; + display: flex; .ant-tooltip-open { - width: 100%; + display: inline; } .truncated { @@ -59,14 +59,6 @@ const StyledCrossLinks = styled.div` `} `; -const StyledLinkedTooltip = styled.div` - .link { - color: ${({ theme }) => theme.colors.grayscale.light5}; - display: block; - text-decoration: underline; - } -`; - export default function CrossLinks({ crossLinks, maxLinks = 50, @@ -75,49 +67,43 @@ export default function CrossLinks({ const crossLinksRef = useRef(null); const [elementsTruncated, hasHiddenElements] = useTruncation(crossLinksRef); + const links = useMemo( + () => + crossLinks.map((link, index) => ( + + {index === 0 ? link.title : `, ${link.title}`} + + )), + [crossLinks], + ); + return ( - {crossLinks.length > 1 ? ( - - {crossLinks.slice(0, maxLinks).map(link => ( - - {link.title} - - ))} - {crossLinks.length > maxLinks && ( - {t('Plus %s more', crossLinks.length - maxLinks)} - )} - - } - > - - {crossLinks.map((element, index) => ( - {index === 0 ? element.title : `, ${element.title}`} - ))} - - {hasHiddenElements && ( - +{elementsTruncated} - )} - - ) : ( - crossLinks[0] && ( - + {elementsTruncated ? ( + maxLinks + ? crossLinks.length - maxLinks + : undefined + } + crossLinks={crossLinks.slice(0, maxLinks).map(l => ({ + title: l.title, + to: linkPrefix + l.id, + }))} > - {crossLinks[0].title} - - ) - )} + {links} + + ) : ( + links + )} + + {hasHiddenElements && +{elementsTruncated}} ); } diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx new file mode 100644 index 0000000000000..d0a21adf3302a --- /dev/null +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx @@ -0,0 +1,88 @@ +/** + * 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 userEvent from '@testing-library/user-event'; +import React from 'react'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import CrossLinksTooltip, { CrossLinksTooltipProps } from './CrossLinksTooltip'; + +const mockedProps = { + crossLinks: [ + { + to: 'somewhere/1', + title: 'Test dashboard', + }, + { + to: 'somewhere/2', + title: 'Test dashboard 2', + }, + { + to: 'somewhere/3', + title: 'Test dashboard 3', + }, + { + to: 'somewhere/4', + title: 'Test dashboard 4', + }, + ], + moreItems: 0, +}; + +function setup(overrideProps: CrossLinksTooltipProps | {} = {}) { + return render( + + Hover me + , + { + useRouter: true, + }, + ); +} + +test('should render', () => { + const { container } = setup(); + expect(container).toBeInTheDocument(); +}); + +test('should render multiple links', async () => { + setup(); + userEvent.hover(screen.getByText('Hover me')); + + await waitFor(() => { + expect(screen.getByText('Test dashboard')).toBeInTheDocument(); + expect(screen.getByText('Test dashboard 2')).toBeInTheDocument(); + expect(screen.getByText('Test dashboard 3')).toBeInTheDocument(); + expect(screen.getByText('Test dashboard 4')).toBeInTheDocument(); + expect(screen.getAllByRole('link')).toHaveLength(4); + }); +}); + +test('should not render the "Plus {x} more"', () => { + setup(); + userEvent.hover(screen.getByText('Hover me')); + expect(screen.queryByTestId('plus-more')).not.toBeInTheDocument(); +}); + +test('should render the "Plus {x} more"', async () => { + setup({ + moreItems: 3, + }); + userEvent.hover(screen.getByText('Hover me')); + expect(await screen.findByTestId('plus-more')).toBeInTheDocument(); + expect(await screen.findByText('Plus 3 more')).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx new file mode 100644 index 0000000000000..e6869c92e24bb --- /dev/null +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx @@ -0,0 +1,69 @@ +/** + * 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 { styled, t } from '@superset-ui/core'; +import { Tooltip } from 'src/components/Tooltip'; +import { Link } from 'react-router-dom'; + +export type CrossLinksTooltipProps = { + children: React.ReactNode; + crossLinks: { to: string; title: string }[]; + moreItems?: number; +}; + +const StyledLinkedTooltip = styled.div` + .link { + color: ${({ theme }) => theme.colors.grayscale.light5}; + display: block; + text-decoration: underline; + } +`; + +export default function CrossLinksTooltip({ + children, + crossLinks = [], + moreItems = undefined, +}: CrossLinksTooltipProps) { + return ( + + {crossLinks.map(link => ( + + {link.title} + + ))} + {moreItems && ( + {t('Plus %s more', moreItems)} + )} + + } + > + {children} + + ); +} diff --git a/superset-frontend/src/hooks/useTruncation/index.ts b/superset-frontend/src/hooks/useTruncation/index.ts index d000a6277afd9..311ad790fde90 100644 --- a/superset-frontend/src/hooks/useTruncation/index.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -68,7 +68,6 @@ export const useTruncation = (elementRef: RefObject) => { width += (childNodes[i] as HTMLElement).offsetWidth; i += 1; } - console.log('i', i); if (i === elementsCount) { setElementsTruncated(1); setHasHiddenElements(false); diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 6376fa4bcbad0..621100f7fb292 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -17,6 +17,7 @@ * under the License. */ import { + ensureIsArray, getChartMetadataRegistry, styled, SupersetClient, @@ -378,10 +379,12 @@ function ChartList(props: ChartListProps) { }, }: any) => ( ({ - title: d.dashboard_title, - id: d.id, - }))} + crossLinks={ensureIsArray(dashboards).map( + (d: ChartLinkedDashboard) => ({ + title: d.dashboard_title, + id: d.id, + }), + )} /> ), Header: t('Dashboards added to'), From 19cf1bf60564226c06953f06736bdf3f0dcd5669 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 10 Oct 2022 16:50:45 +0300 Subject: [PATCH 04/15] Add feature flag --- .../src/views/CRUD/chart/ChartList.tsx | 70 +++++++++++-------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 621100f7fb292..b1a43b1f19e0b 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -265,6 +265,7 @@ function ChartList(props: ChartListProps) { const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; const enableBroadUserAccess = bootstrapData?.common?.conf?.ENABLE_BROAD_ACTIVITY_ACCESS; + const enableCrossRef = isFeatureEnabled(FeatureFlag.CROSS_REFERENCES); const handleBulkChartExport = (chartsToExport: Chart[]) => { const ids = chartsToExport.map(({ id }) => id); handleResourceExport('chart', ids, () => { @@ -295,6 +296,30 @@ function ChartList(props: ChartListProps) { ); } + const dashboardsCol = useMemo( + () => ({ + Cell: ({ + row: { + original: { dashboards }, + }, + }: any) => ( + ({ + title: d.dashboard_title, + id: d.id, + }), + )} + /> + ), + Header: t('Dashboards added to'), + accessor: 'dashboards', + disableSortBy: true, + size: 'xxl', + }), + [], + ); + const columns = useMemo( () => [ { @@ -372,26 +397,7 @@ function ChartList(props: ChartListProps) { disableSortBy: true, size: 'xl', }, - { - Cell: ({ - row: { - original: { dashboards }, - }, - }: any) => ( - ({ - title: d.dashboard_title, - id: d.id, - }), - )} - /> - ), - Header: t('Dashboards added to'), - accessor: 'dashboards', - disableSortBy: true, - size: 'xxl', - }, + ...(enableCrossRef ? [dashboardsCol] : []), { Cell: ({ row: { @@ -558,6 +564,19 @@ function ChartList(props: ChartListProps) { [], ); + const dashboardsFilter: Filter = useMemo( + () => ({ + Header: t('Dashboards'), + id: 'dashboards', + input: 'select', + operator: FilterOperator.relationManyMany, + unfilteredLabel: t('All'), + fetchSelects: createFetchDashboards, + paginate: true, + }), + [], + ); + const filters: Filters = useMemo( () => [ { @@ -636,15 +655,7 @@ function ChartList(props: ChartListProps) { fetchSelects: createFetchDatasets, paginate: true, }, - { - Header: t('Dashboards'), - id: 'dashboards', - input: 'select', - operator: FilterOperator.relationManyMany, - unfilteredLabel: t('All'), - fetchSelects: createFetchDashboards, - paginate: true, - }, + ...(enableCrossRef ? [dashboardsFilter] : []), ...(userId ? [favoritesFilter] : []), { Header: t('Certified'), @@ -759,6 +770,7 @@ function ChartList(props: ChartListProps) { }); } } + return ( <> From 515018e807069a22f8afc26b52300e19c6ee5e00 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 10 Oct 2022 17:01:23 +0300 Subject: [PATCH 05/15] Enhance E2E tests --- .../cypress/integration/chart_list/filter.test.ts | 14 ++++++++++++++ .../cypress/integration/chart_list/list.test.ts | 9 +++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts index eff415107901e..9ba7a4a29e825 100644 --- a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts @@ -62,6 +62,13 @@ describe('Charts filters', () => { setFilter('Dataset', 'unicode_test'); cy.getBySel('styled-card').should('have.length', 1); }); + + it('should filter by dashboards correctly', () => { + setFilter('Dashboards', 'Unicode Test'); + cy.getBySel('styled-card').should('have.length', 1); + setFilter('Dashboards', 'Tabbed Dashboard'); + cy.getBySel('styled-card').should('have.length', 9); + }); }); describe('list-view', () => { @@ -96,5 +103,12 @@ describe('Charts filters', () => { setFilter('Dataset', 'unicode_test'); cy.getBySel('table-row').should('have.length', 1); }); + + it('should filter by dashboards correctly', () => { + setFilter('Dashboards', 'Unicode Test'); + cy.getBySel('table-row').should('have.length', 1); + setFilter('Dashboards', 'Tabbed Dashboard'); + cy.getBySel('table-row').should('have.length', 9); + }); }); }); diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts index e3837445d9522..6981ead73ab9b 100644 --- a/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/chart_list/list.test.ts @@ -59,10 +59,11 @@ describe('Charts list', () => { cy.getBySel('sort-header').eq(1).contains('Chart'); cy.getBySel('sort-header').eq(2).contains('Visualization type'); cy.getBySel('sort-header').eq(3).contains('Dataset'); - cy.getBySel('sort-header').eq(4).contains('Modified by'); - cy.getBySel('sort-header').eq(5).contains('Last modified'); - cy.getBySel('sort-header').eq(6).contains('Created by'); - cy.getBySel('sort-header').eq(7).contains('Actions'); + cy.getBySel('sort-header').eq(4).contains('Dashboards added to'); + cy.getBySel('sort-header').eq(5).contains('Modified by'); + cy.getBySel('sort-header').eq(6).contains('Last modified'); + cy.getBySel('sort-header').eq(7).contains('Created by'); + cy.getBySel('sort-header').eq(8).contains('Actions'); }); it('should sort correctly in list mode', () => { From 9a7301d2b533ff4a40fce939463c804517411d90 Mon Sep 17 00:00:00 2001 From: geido Date: Mon, 10 Oct 2022 17:11:03 +0300 Subject: [PATCH 06/15] Reduce maxLinks --- superset-frontend/src/components/ListView/CrossLinks.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index aa1796a6c784d..bfe6502cc1a83 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -61,7 +61,7 @@ const StyledCrossLinks = styled.div` export default function CrossLinks({ crossLinks, - maxLinks = 50, + maxLinks = 20, linkPrefix = '/superset/dashboard/', }: CrossLinksProps) { const crossLinksRef = useRef(null); From c82d84977b553c43665fa4f104420eeb7c36440c Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Tue, 11 Oct 2022 14:46:14 +0300 Subject: [PATCH 07/15] Update superset-frontend/src/views/CRUD/chart/ChartList.tsx Co-authored-by: Kamil Gabryjelski --- superset-frontend/src/views/CRUD/chart/ChartList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index b1a43b1f19e0b..b7e9a17d08e57 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -144,7 +144,7 @@ const createFetchDashboards = async ( ) => { // add filters if filterValue const filters = filterValue - ? { filters: [{ col: 'dashboards', opr: 'rel_m_m', value: filterValue }] } + ? { filters: [{ col: 'dashboards', opr: FilterOperator.relationManyMany, value: filterValue }] } : {}; const queryParams = rison.encode({ columns: ['dashboard_title', 'id'], From ede97f2dc4561f70445bfc8e0eeee9d09f20ab9f Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 11 Oct 2022 15:06:54 +0300 Subject: [PATCH 08/15] Update tests --- .../cypress/integration/chart_list/filter.test.ts | 4 ++-- .../src/components/ListView/CrossLinksTooltip.test.tsx | 6 +++--- .../src/components/ListView/CrossLinksTooltip.tsx | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts index 9ba7a4a29e825..7bd0891cbf268 100644 --- a/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/chart_list/filter.test.ts @@ -67,7 +67,7 @@ describe('Charts filters', () => { setFilter('Dashboards', 'Unicode Test'); cy.getBySel('styled-card').should('have.length', 1); setFilter('Dashboards', 'Tabbed Dashboard'); - cy.getBySel('styled-card').should('have.length', 9); + cy.getBySel('styled-card').should('have.length', 8); }); }); @@ -108,7 +108,7 @@ describe('Charts filters', () => { setFilter('Dashboards', 'Unicode Test'); cy.getBySel('table-row').should('have.length', 1); setFilter('Dashboards', 'Tabbed Dashboard'); - cy.getBySel('table-row').should('have.length', 9); + cy.getBySel('table-row').should('have.length', 8); }); }); }); diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx index d0a21adf3302a..d95318a89226e 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx @@ -72,17 +72,17 @@ test('should render multiple links', async () => { }); }); -test('should not render the "Plus {x} more"', () => { +test('should not render the "+ {x} more"', () => { setup(); userEvent.hover(screen.getByText('Hover me')); expect(screen.queryByTestId('plus-more')).not.toBeInTheDocument(); }); -test('should render the "Plus {x} more"', async () => { +test('should render the "+ {x} more"', async () => { setup({ moreItems: 3, }); userEvent.hover(screen.getByText('Hover me')); expect(await screen.findByTestId('plus-more')).toBeInTheDocument(); - expect(await screen.findByText('Plus 3 more')).toBeInTheDocument(); + expect(await screen.findByText('+ 3 more')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx index e6869c92e24bb..a6d7cce541aca 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx @@ -58,7 +58,7 @@ export default function CrossLinksTooltip({ ))} {moreItems && ( - {t('Plus %s more', moreItems)} + {t('+ %s more', moreItems)} )} } From 78b41f7631ea147de124189b4bfc6ef21ab32bd8 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 11 Oct 2022 17:49:16 +0300 Subject: [PATCH 09/15] Minor enhancements --- .../components/ListView/CrossLinks.test.tsx | 6 +- .../ListView/CrossLinksTooltip.test.tsx | 2 +- .../src/views/CRUD/chart/ChartList.tsx | 100 ++++++++++-------- 3 files changed, 59 insertions(+), 49 deletions(-) diff --git a/superset-frontend/src/components/ListView/CrossLinks.test.tsx b/superset-frontend/src/components/ListView/CrossLinks.test.tsx index e50b32fe35c9a..ad7eb4e0dd281 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.test.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.test.tsx @@ -42,7 +42,7 @@ const mockedProps = { }; function setup(overrideProps: CrossLinksProps | {} = {}) { - return render(, { + return render(, { useRouter: true, }); } @@ -83,11 +83,11 @@ test('should render a custom prefix link', () => { title: 'Test dashboard', }, ], - linkPrefix: '/preset/dashboard/', + linkPrefix: '/custom/dashboard/', }); expect(screen.getByRole('link')).toHaveAttribute( 'href', - `/preset/dashboard/1`, + `/custom/dashboard/1`, ); }); diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx index d95318a89226e..b0c40513ed760 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx @@ -45,7 +45,7 @@ const mockedProps = { function setup(overrideProps: CrossLinksTooltipProps | {} = {}) { return render( - + Hover me , { diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index b7e9a17d08e57..cf721ff73603d 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,6 +19,7 @@ import { ensureIsArray, getChartMetadataRegistry, + JsonObject, styled, SupersetClient, t, @@ -137,47 +138,6 @@ const createFetchDatasets = async ( }; }; -const createFetchDashboards = async ( - filterValue = '', - page: number, - pageSize: number, -) => { - // add filters if filterValue - const filters = filterValue - ? { filters: [{ col: 'dashboards', opr: FilterOperator.relationManyMany, value: filterValue }] } - : {}; - const queryParams = rison.encode({ - columns: ['dashboard_title', 'id'], - keys: ['none'], - order_column: 'dashboard_title', - order_direction: 'asc', - page, - page_size: pageSize, - ...filters, - }); - const { json = {} } = await SupersetClient.get({ - endpoint: !filterValue - ? `/api/v1/dashboard/?q=${queryParams}` - : `/api/v1/chart/?q=${queryParams}`, - }); - const dashboards = json?.result?.map( - ({ - dashboard_title: dashboardTitle, - id, - }: { - dashboard_title: string; - id: number; - }) => ({ - label: dashboardTitle, - value: id, - }), - ); - return { - data: uniqBy(dashboards, 'value'), - totalCount: json?.count, - }; -}; - interface ChartListProps { addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; @@ -265,7 +225,7 @@ function ChartList(props: ChartListProps) { const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; const enableBroadUserAccess = bootstrapData?.common?.conf?.ENABLE_BROAD_ACTIVITY_ACCESS; - const enableCrossRef = isFeatureEnabled(FeatureFlag.CROSS_REFERENCES); + const crossRefEnabled = isFeatureEnabled(FeatureFlag.CROSS_REFERENCES); const handleBulkChartExport = (chartsToExport: Chart[]) => { const ids = chartsToExport.map(({ id }) => id); handleResourceExport('chart', ids, () => { @@ -295,6 +255,56 @@ function ChartList(props: ChartListProps) { ), ); } + const fetchDashboards = async ( + filterValue = '', + page: number, + pageSize: number, + ) => { + // add filters if filterValue + const filters = filterValue + ? { + filters: [ + { + col: 'dashboards', + opr: FilterOperator.relationManyMany, + value: filterValue, + }, + ], + } + : {}; + const queryParams = rison.encode({ + columns: ['dashboard_title', 'id'], + keys: ['none'], + order_column: 'dashboard_title', + order_direction: 'asc', + page, + page_size: pageSize, + ...filters, + }); + const json: void | JsonObject = await SupersetClient.get({ + endpoint: !filterValue + ? `/api/v1/dashboard/?q=${queryParams}` + : `/api/v1/chart/?q=${queryParams}`, + }).catch(() => + addDangerToast(t('An error occurred while fetching dashboards')), + ); + const dashboards = json?.result?.map( + ({ + dashboard_title: dashboardTitle, + id, + }: { + dashboard_title: string; + id: number; + }) => ({ + label: dashboardTitle, + value: id, + }), + ); + return { + data: uniqBy(dashboards, 'value'), + totalCount: json?.count, + }; + }; const dashboardsCol = useMemo( () => ({ @@ -397,7 +407,7 @@ function ChartList(props: ChartListProps) { disableSortBy: true, size: 'xl', }, - ...(enableCrossRef ? [dashboardsCol] : []), + ...(crossRefEnabled ? [dashboardsCol] : []), { Cell: ({ row: { @@ -571,7 +581,7 @@ function ChartList(props: ChartListProps) { input: 'select', operator: FilterOperator.relationManyMany, unfilteredLabel: t('All'), - fetchSelects: createFetchDashboards, + fetchSelects: fetchDashboards, paginate: true, }), [], @@ -655,7 +665,7 @@ function ChartList(props: ChartListProps) { fetchSelects: createFetchDatasets, paginate: true, }, - ...(enableCrossRef ? [dashboardsFilter] : []), + ...(crossRefEnabled ? [dashboardsFilter] : []), ...(userId ? [favoritesFilter] : []), { Header: t('Certified'), From 7bb8a0d719af98315728cb3f2e454e57860e4570 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 11 Oct 2022 19:31:22 +0300 Subject: [PATCH 10/15] Fix dashboards fetch --- superset-frontend/src/views/CRUD/chart/ChartList.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index cf721ff73603d..7600dfbf5d6bf 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -19,7 +19,7 @@ import { ensureIsArray, getChartMetadataRegistry, - JsonObject, + JsonResponse, styled, SupersetClient, t, @@ -281,14 +281,14 @@ function ChartList(props: ChartListProps) { page_size: pageSize, ...filters, }); - const json: void | JsonObject = await SupersetClient.get({ + const response: void | JsonResponse = await SupersetClient.get({ endpoint: !filterValue ? `/api/v1/dashboard/?q=${queryParams}` : `/api/v1/chart/?q=${queryParams}`, }).catch(() => addDangerToast(t('An error occurred while fetching dashboards')), ); - const dashboards = json?.result?.map( + const dashboards = response?.json?.result?.map( ({ dashboard_title: dashboardTitle, id, @@ -302,7 +302,7 @@ function ChartList(props: ChartListProps) { ); return { data: uniqBy(dashboards, 'value'), - totalCount: json?.count, + totalCount: response?.json?.count, }; }; From 465cc6c05d6ff7a19aad60150355deb1a6e5c298 Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 12 Oct 2022 17:14:23 +0300 Subject: [PATCH 11/15] Show tooltip on count --- .../src/components/ListView/CrossLinks.tsx | 104 +++++++++--------- .../src/hooks/useTruncation/index.ts | 23 ++-- 2 files changed, 68 insertions(+), 59 deletions(-) diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index bfe6502cc1a83..cf58f5864d92e 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -35,26 +35,27 @@ export type CrossLinksProps = { const StyledCrossLinks = styled.div` ${({ theme }) => ` - cursor: pointer; - position: relative; - display: flex; + & > span { + width: 100%; + display: flex; - .ant-tooltip-open { - display: inline; - } + .ant-tooltip-open { + display: inline; + } - .truncated { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - display: inline-block; - width: 100%; - vertical-align: bottom; - } + .truncated { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + display: inline-block; + width: 100%; + vertical-align: bottom; + } - .count { - color: ${theme.colors.grayscale.base}; - font-weight: ${theme.typography.weights.bold}; + .count { + color: ${theme.colors.grayscale.base}; + font-weight: ${theme.typography.weights.bold}; + } } `} `; @@ -66,44 +67,49 @@ export default function CrossLinks({ }: CrossLinksProps) { const crossLinksRef = useRef(null); const [elementsTruncated, hasHiddenElements] = useTruncation(crossLinksRef); - - const links = useMemo( + const hasMoreItems = useMemo( () => - crossLinks.map((link, index) => ( - - {index === 0 ? link.title : `, ${link.title}`} - - )), + crossLinks.length > maxLinks ? crossLinks.length - maxLinks : undefined, + [crossLinks, maxLinks], + ); + const links = useMemo( + () => ( + + {crossLinks.map((link, index) => ( + + {index === 0 ? link.title : `, ${link.title}`} + + ))} + + ), [crossLinks], ); + const tooltipLinks = useMemo( + () => + crossLinks.slice(0, maxLinks).map(l => ({ + title: l.title, + to: linkPrefix + l.id, + })), + [crossLinks, maxLinks], + ); return ( - - {elementsTruncated ? ( - maxLinks - ? crossLinks.length - maxLinks - : undefined - } - crossLinks={crossLinks.slice(0, maxLinks).map(l => ({ - title: l.title, - to: linkPrefix + l.id, - }))} - > - {links} - - ) : ( - links - )} - - {hasHiddenElements && +{elementsTruncated}} + {elementsTruncated ? ( + + {links} + {hasHiddenElements && ( + +{elementsTruncated} + )} + + ) : ( + links + )} ); } diff --git a/superset-frontend/src/hooks/useTruncation/index.ts b/superset-frontend/src/hooks/useTruncation/index.ts index 311ad790fde90..4861d5673f395 100644 --- a/superset-frontend/src/hooks/useTruncation/index.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -59,22 +59,25 @@ export const useTruncation = (elementRef: RefObject) => { if (scrollWidth > clientWidth) { // "..." is around 6px wide - const maxWidth = clientWidth - 6; + const truncationWidthLimit = 6; + const maxWidth = clientWidth - truncationWidthLimit; const elementsCount = childNodes.length; let width = 0; - let i = 0; - while (width < maxWidth) { - width += (childNodes[i] as HTMLElement).offsetWidth; - i += 1; + let truncatedElements = 1; + for (let i = 0; i < elementsCount; i += 1) { + const itemWidth = (childNodes[i] as HTMLElement).offsetWidth; + width += itemWidth; + // assures it shows +{number} also when the item is barely visible + if (width > maxWidth + itemWidth / 2) { + truncatedElements += 1; + } } - if (i === elementsCount) { - setElementsTruncated(1); - setHasHiddenElements(false); - } else { - setElementsTruncated(elementsCount - i); + + if (truncatedElements > 1) { setHasHiddenElements(true); } + setElementsTruncated(truncatedElements); } else { setElementsTruncated(0); } From 53521b02a1a4e4c7ba212c15a7e854ab9da1d379 Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 12 Oct 2022 17:37:36 +0300 Subject: [PATCH 12/15] Improve truncation --- superset-frontend/src/hooks/useTruncation/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/hooks/useTruncation/index.ts b/superset-frontend/src/hooks/useTruncation/index.ts index 4861d5673f395..5818414abd76b 100644 --- a/superset-frontend/src/hooks/useTruncation/index.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -64,20 +64,22 @@ export const useTruncation = (elementRef: RefObject) => { const elementsCount = childNodes.length; let width = 0; - let truncatedElements = 1; + let hiddenElements = 0; for (let i = 0; i < elementsCount; i += 1) { const itemWidth = (childNodes[i] as HTMLElement).offsetWidth; width += itemWidth; - // assures it shows +{number} also when the item is barely visible - if (width > maxWidth + itemWidth / 2) { - truncatedElements += 1; + // assures it shows +{number} the item is not visible + if (width > maxWidth + itemWidth / 1.5) { + hiddenElements += 1; } } - if (truncatedElements > 1) { + if (hiddenElements) { setHasHiddenElements(true); + setElementsTruncated(hiddenElements); + } else { + setElementsTruncated(1); } - setElementsTruncated(truncatedElements); } else { setElementsTruncated(0); } From 58c0d8cfb384178f1a88085568ac2bd0dad8c484 Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 12 Oct 2022 17:53:18 +0300 Subject: [PATCH 13/15] Minor improvements --- superset-frontend/src/components/ListView/CrossLinks.tsx | 1 + superset-frontend/src/components/ListView/CrossLinksTooltip.tsx | 2 +- superset-frontend/src/hooks/useTruncation/index.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index cf58f5864d92e..fa90d73cf2854 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -53,6 +53,7 @@ const StyledCrossLinks = styled.div` } .count { + cursor: pointer; color: ${theme.colors.grayscale.base}; font-weight: ${theme.typography.weights.bold}; } diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx index a6d7cce541aca..46ab4c9ac016b 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx @@ -42,7 +42,7 @@ export default function CrossLinksTooltip({ }: CrossLinksTooltipProps) { return ( diff --git a/superset-frontend/src/hooks/useTruncation/index.ts b/superset-frontend/src/hooks/useTruncation/index.ts index 5818414abd76b..3b356f0660f01 100644 --- a/superset-frontend/src/hooks/useTruncation/index.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -74,7 +74,7 @@ export const useTruncation = (elementRef: RefObject) => { } } - if (hiddenElements) { + if (elementsCount > 1 && hiddenElements) { setHasHiddenElements(true); setElementsTruncated(hiddenElements); } else { From 195b50c04c95bb5ff307981731373aa852ef844e Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 12 Oct 2022 19:30:22 +0300 Subject: [PATCH 14/15] Refactor --- .../src/components/ListView/CrossLinks.tsx | 28 +++++++++------ .../ListView/CrossLinksTooltip.test.tsx | 1 + .../components/ListView/CrossLinksTooltip.tsx | 36 ++++++++++--------- .../FilterCard/DependenciesRow.tsx | 5 +-- .../nativeFilters/FilterCard/ScopeRow.tsx | 5 +-- .../src/hooks/useTruncation/index.ts | 28 +++++++++++---- 6 files changed, 65 insertions(+), 38 deletions(-) diff --git a/superset-frontend/src/components/ListView/CrossLinks.tsx b/superset-frontend/src/components/ListView/CrossLinks.tsx index fa90d73cf2854..3941bcf6caac1 100644 --- a/superset-frontend/src/components/ListView/CrossLinks.tsx +++ b/superset-frontend/src/components/ListView/CrossLinks.tsx @@ -67,7 +67,11 @@ export default function CrossLinks({ linkPrefix = '/superset/dashboard/', }: CrossLinksProps) { const crossLinksRef = useRef(null); - const [elementsTruncated, hasHiddenElements] = useTruncation(crossLinksRef); + const plusRef = useRef(null); + const [elementsTruncated, hasHiddenElements] = useTruncation( + crossLinksRef, + plusRef, + ); const hasMoreItems = useMemo( () => crossLinks.length > maxLinks ? crossLinks.length - maxLinks : undefined, @@ -101,16 +105,18 @@ export default function CrossLinks({ return ( - {elementsTruncated ? ( - - {links} - {hasHiddenElements && ( - +{elementsTruncated} - )} - - ) : ( - links - )} + + {links} + {hasHiddenElements && ( + + +{elementsTruncated} + + )} + ); } diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx index b0c40513ed760..96723e7bf698d 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.test.tsx @@ -41,6 +41,7 @@ const mockedProps = { }, ], moreItems: 0, + show: true, }; function setup(overrideProps: CrossLinksTooltipProps | {} = {}) { diff --git a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx index 46ab4c9ac016b..cc552cd8b4cf9 100644 --- a/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx +++ b/superset-frontend/src/components/ListView/CrossLinksTooltip.tsx @@ -25,6 +25,7 @@ export type CrossLinksTooltipProps = { children: React.ReactNode; crossLinks: { to: string; title: string }[]; moreItems?: number; + show: boolean; }; const StyledLinkedTooltip = styled.div` @@ -39,28 +40,31 @@ export default function CrossLinksTooltip({ children, crossLinks = [], moreItems = undefined, + show = false, }: CrossLinksTooltipProps) { return ( - {crossLinks.map(link => ( - - {link.title} - - ))} - {moreItems && ( - {t('+ %s more', moreItems)} - )} - + show && ( + + {crossLinks.map(link => ( + + {link.title} + + ))} + {moreItems && ( + {t('+ %s more', moreItems)} + )} + + ) } > {children} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx index abbaa1e63357c..5486651a46d5b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx @@ -55,7 +55,8 @@ const DependencyValue = ({ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { const dependencies = useFilterDependencies(filter); const dependenciesRef = useRef(null); - const [elementsTruncated, hasHiddenElements] = useTruncation(dependenciesRef); + const plusRef = useRef(null); + const [elementsTruncated, hasHiddenElements] = useTruncation(dependenciesRef, plusRef); const theme = useTheme(); const tooltipText = useMemo( @@ -108,7 +109,7 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { ))} {hasHiddenElements && ( - +{elementsTruncated} + +{elementsTruncated} )} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx index 6a672ed42c90e..282a2897b3471 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx @@ -46,8 +46,9 @@ const getTooltipSection = (items: string[] | undefined, label: string) => export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { const scope = useFilterScope(filter); const scopeRef = useRef(null); + const plusRef = useRef(null); - const [elementsTruncated, hasHiddenElements] = useTruncation(scopeRef); + const [elementsTruncated, hasHiddenElements] = useTruncation(scopeRef, plusRef); const tooltipText = useMemo(() => { if (elementsTruncated === 0 || !scope) { return null; @@ -77,7 +78,7 @@ export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { : t('None')} {hasHiddenElements > 0 && ( - +{elementsTruncated} + +{elementsTruncated} )} diff --git a/superset-frontend/src/hooks/useTruncation/index.ts b/superset-frontend/src/hooks/useTruncation/index.ts index 3b356f0660f01..7f3e1bcadecee 100644 --- a/superset-frontend/src/hooks/useTruncation/index.ts +++ b/superset-frontend/src/hooks/useTruncation/index.ts @@ -18,17 +18,23 @@ */ import { RefObject, useLayoutEffect, useState, useRef } from 'react'; -export const useTruncation = (elementRef: RefObject) => { +export const useTruncation = ( + elementRef: RefObject, + plusRef?: RefObject, +) => { const [elementsTruncated, setElementsTruncated] = useState(0); const [hasHiddenElements, setHasHiddenElements] = useState(false); const previousEffectInfoRef = useRef({ scrollWidth: 0, parentElementWidth: 0, + plusRefWidth: 0, }); useLayoutEffect(() => { const currentElement = elementRef.current; + const plusRefElement = plusRef?.current; + if (!currentElement) { return; } @@ -45,42 +51,50 @@ export const useTruncation = (elementRef: RefObject) => { // the child nodes changes. const previousEffectInfo = previousEffectInfoRef.current; const parentElementWidth = currentElement.parentElement?.clientWidth || 0; + const plusRefWidth = plusRefElement?.offsetWidth || 0; previousEffectInfoRef.current = { scrollWidth, parentElementWidth, + plusRefWidth, }; if ( previousEffectInfo.parentElementWidth === parentElementWidth && - previousEffectInfo.scrollWidth === scrollWidth + previousEffectInfo.scrollWidth === scrollWidth && + previousEffectInfo.plusRefWidth === plusRefWidth ) { return; } if (scrollWidth > clientWidth) { // "..." is around 6px wide - const truncationWidthLimit = 6; - const maxWidth = clientWidth - truncationWidthLimit; + const truncationWidth = 6; + const plusSize = plusRefElement?.offsetWidth || 0; + const maxWidth = clientWidth - truncationWidth; const elementsCount = childNodes.length; let width = 0; let hiddenElements = 0; for (let i = 0; i < elementsCount; i += 1) { const itemWidth = (childNodes[i] as HTMLElement).offsetWidth; - width += itemWidth; - // assures it shows +{number} the item is not visible - if (width > maxWidth + itemWidth / 1.5) { + const remainingWidth = maxWidth - truncationWidth - width - plusSize; + + // assures it shows +{number} only when the item is not visible + if (remainingWidth <= 0) { hiddenElements += 1; } + width += itemWidth; } if (elementsCount > 1 && hiddenElements) { setHasHiddenElements(true); setElementsTruncated(hiddenElements); } else { + setHasHiddenElements(false); setElementsTruncated(1); } } else { + setHasHiddenElements(false); setElementsTruncated(0); } }, [ From 7774d5c4d95bbd558e21fc5d0d0236ea3336de66 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 13 Oct 2022 09:45:57 +0300 Subject: [PATCH 15/15] Pretty --- .../nativeFilters/FilterCard/DependenciesRow.tsx | 9 +++++++-- .../components/nativeFilters/FilterCard/ScopeRow.tsx | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx index 5486651a46d5b..704357c134868 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx @@ -56,7 +56,10 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { const dependencies = useFilterDependencies(filter); const dependenciesRef = useRef(null); const plusRef = useRef(null); - const [elementsTruncated, hasHiddenElements] = useTruncation(dependenciesRef, plusRef); + const [elementsTruncated, hasHiddenElements] = useTruncation( + dependenciesRef, + plusRef, + ); const theme = useTheme(); const tooltipText = useMemo( @@ -109,7 +112,9 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { ))} {hasHiddenElements && ( - +{elementsTruncated} + + +{elementsTruncated} + )} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx index 282a2897b3471..8da224c0e706d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx @@ -48,7 +48,10 @@ export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { const scopeRef = useRef(null); const plusRef = useRef(null); - const [elementsTruncated, hasHiddenElements] = useTruncation(scopeRef, plusRef); + const [elementsTruncated, hasHiddenElements] = useTruncation( + scopeRef, + plusRef, + ); const tooltipText = useMemo(() => { if (elementsTruncated === 0 || !scope) { return null; @@ -78,7 +81,9 @@ export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { : t('None')} {hasHiddenElements > 0 && ( - +{elementsTruncated} + + +{elementsTruncated} + )}