diff --git a/workspaces/scorecard/.changeset/dirty-turtles-pick.md b/workspaces/scorecard/.changeset/dirty-turtles-pick.md new file mode 100644 index 0000000000..6846ff0007 --- /dev/null +++ b/workspaces/scorecard/.changeset/dirty-turtles-pick.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard': patch +--- + +Fix tooltip display and error title clipping for some languages diff --git a/workspaces/scorecard/packages/app/e2e-tests/pages/HomePage.ts b/workspaces/scorecard/packages/app/e2e-tests/pages/HomePage.ts index 94ee05f01a..3b0431da1f 100644 --- a/workspaces/scorecard/packages/app/e2e-tests/pages/HomePage.ts +++ b/workspaces/scorecard/packages/app/e2e-tests/pages/HomePage.ts @@ -93,7 +93,7 @@ export class HomePage { ) { const card = this.getCard(metricId); await expect(card).toContainText( - this.translations.errors.missingPermissionMessage, + this.translations.errors.missingPermission, ); } @@ -102,8 +102,6 @@ export class HomePage { ) { const card = this.getCard(metricId); await expect(card).toContainText(this.translations.errors.noDataFound); - await expect(card).toContainText( - this.translations.errors.noDataFoundMessage, - ); + await expect(card).toContainText(this.translations.errors.noDataFound); } } diff --git a/workspaces/scorecard/plugins/scorecard/src/components/Common/ErrorTooltip.tsx b/workspaces/scorecard/plugins/scorecard/src/components/Common/ErrorTooltip.tsx new file mode 100644 index 0000000000..e6d50c03b6 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard/src/components/Common/ErrorTooltip.tsx @@ -0,0 +1,40 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed 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 MuiTooltip from '@mui/material/Tooltip'; + +export const ErrorTooltip = ({ + title, + tooltipPosition, +}: { + title: string | undefined; + tooltipPosition: { x: number; y: number } | undefined; +}) => { + return ( +
+ + {/* Anchor for tooltip so it appears under the pie chart */} +
Tooltip
+
+
+ ); +}; diff --git a/workspaces/scorecard/plugins/scorecard/src/components/Common/__tests__/ErrorTooltip.test.tsx b/workspaces/scorecard/plugins/scorecard/src/components/Common/__tests__/ErrorTooltip.test.tsx new file mode 100644 index 0000000000..47b45a21f3 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard/src/components/Common/__tests__/ErrorTooltip.test.tsx @@ -0,0 +1,73 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed 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 { render, screen } from '@testing-library/react'; + +import { ErrorTooltip } from '../ErrorTooltip'; + +// MUI Tooltip uses portals + transitions +// disable them for stable tests +jest.mock('@mui/material/Tooltip', () => { + return ({ children, title, open }: any) => ( +
+ {children} + {open && title && {title}} +
+ ); +}); + +describe('ErrorTooltip Component', () => { + it('should render tooltip when title is provided', () => { + render( + , + ); + + expect(screen.getByTestId('tooltip')).toHaveTextContent('Error occurred'); + }); + + it('should position tooltip correctly', () => { + const { container } = render( + , + ); + + const wrapper = container.firstChild as HTMLElement; + + expect(wrapper).toHaveStyle({ + left: '50px', + top: '75px', + position: 'absolute', + }); + }); + + it('should not render tooltip text when title is undefined', () => { + render( + , + ); + + expect(screen.queryByTestId('tooltip')).not.toBeInTheDocument(); + }); + + it('should render even if tooltipPosition is undefined', () => { + const { container } = render( + , + ); + + expect(container.firstChild).toBeInTheDocument(); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard/src/components/Scorecard/Scorecard.tsx b/workspaces/scorecard/plugins/scorecard/src/components/Scorecard/Scorecard.tsx index 6b793b6b87..1e7e1469f3 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/Scorecard/Scorecard.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/Scorecard/Scorecard.tsx @@ -14,6 +14,8 @@ * limitations under the License. */ +import { useLayoutEffect, useRef, useState } from 'react'; + import { MetricValue, ThresholdResult, @@ -27,14 +29,18 @@ import { Legend, Tooltip, } from 'recharts'; - import Box from '@mui/material/Box'; import { useTheme } from '@mui/material/styles'; -import MuiTooltip from '@mui/material/Tooltip'; + import { useTranslation } from '../../hooks/useTranslation'; import { CardWrapper } from '../Common/CardWrapper'; import CustomLegend from './CustomLegend'; -import { getRingColor } from '../../utils/utils'; +import { + getHeightForCenterLabel, + getRingColor, + getYOffsetForCenterLabel, +} from '../../utils/utils'; +import { ErrorTooltip } from '../Common/ErrorTooltip'; interface ScorecardProps { cardTitle: string; @@ -49,6 +55,108 @@ interface ScorecardProps { thresholdError?: string; } +const ScorecardCenterLabel = ({ + cx, + cy, + statusColor, + StatusIcon, + value, + isErrorState, + errorLabel, + color, + onLabelMouseEnter, + onLabelMouseLeave, +}: { + cx: number; + cy: number; + statusColor: string; + StatusIcon: React.ElementType; + value: MetricValue | null; + isErrorState: boolean; + errorLabel: string; + color: string | undefined; + onLabelMouseEnter: (e: React.MouseEvent) => void; + onLabelMouseLeave: (e: React.MouseEvent) => void; +}) => { + const fontSize = 14; + const lineHeight = 1.2; + + const textRef = useRef(null); + const [layout, setLayout] = useState({ yOffset: -10, height: 40 }); + + useLayoutEffect(() => { + if (!isErrorState) return; + const el = textRef.current; + if (!el) return; + + const lineHeightPx = fontSize * lineHeight; + const lineCount = Math.round(el.scrollHeight / lineHeightPx); + + const nextOffset = getYOffsetForCenterLabel(lineCount); + const nextHeight = getHeightForCenterLabel(lineCount); + + setLayout(prev => + prev.yOffset === nextOffset && prev.height === nextHeight + ? prev + : { yOffset: nextOffset, height: nextHeight }, + ); + }, [isErrorState, errorLabel]); + + return ( + + + + muiTheme.palette[statusColor.split('.')[0]][ + statusColor.split('.')[1] + ], + }} + /> + + {!isErrorState && ( + + {value} + + )} + {isErrorState && ( + +
+ {errorLabel} +
+
+ )} +
+ ); +}; + const Scorecard = ({ cardTitle, description, @@ -64,6 +172,8 @@ const Scorecard = ({ const theme = useTheme(); const { t } = useTranslation(); + const [isPieAreaActive, setIsPieAreaActive] = useState(false); + const isErrorState = isMetricDataError || isThresholdError; const ringColor = getRingColor(theme, statusColor, isErrorState); @@ -100,6 +210,27 @@ const Scorecard = ({ }} > + {/* This is the circle that is used to trigger the tooltip */} + {isErrorState && ( + + { + setIsPieAreaActive(true); + e.stopPropagation(); + }} + onMouseLeave={e => { + setIsPieAreaActive(false); + e.stopPropagation(); + }} + /> + + )} + - - - muiTheme.palette[statusColor.split('.')[0]][ - statusColor.split('.')[1] - ], - }} - /> - - {!isErrorState && ( - - {value} - - )} - - {isErrorState && ( - -
- {isMetricDataError && - t('errors.metricDataUnavailable')} - {!isMetricDataError && - isThresholdError && - t('errors.invalidThresholds')} -
-
- )} - + { + setIsPieAreaActive(true); + e.stopPropagation(); + }} + onLabelMouseLeave={e => { + setIsPieAreaActive(false); + e.stopPropagation(); + }} + /> ); }} + onMouseEnter={() => { + setIsPieAreaActive(true); + }} + onMouseLeave={() => { + setIsPieAreaActive(false); + }} > {pieData.map(entry => ( @@ -210,35 +325,26 @@ const Scorecard = ({ /> { - if (!active) return null; + content={({ coordinate }) => { + let errorTooltipTitle: string | undefined; + if (isMetricDataError) { + errorTooltipTitle = metricDataError; + } else if (isThresholdError) { + errorTooltipTitle = thresholdError; + } else { + errorTooltipTitle = undefined; + } + if (!isPieAreaActive || coordinate === undefined) return null; return ( - - {/* Need to hide the tooltip content because we are using the position prop to position the tooltip */} -
Tooltip content
-
+ /> ); }} /> diff --git a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/EmptyStatePanel.tsx b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/EmptyStatePanel.tsx index 7446017003..bfebb60456 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/EmptyStatePanel.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/EmptyStatePanel.tsx @@ -20,9 +20,14 @@ import { useTheme } from '@mui/material/styles'; import { CardWrapper } from '../Common/CardWrapper'; import { useTranslation } from '../../hooks/useTranslation'; -import { getStatusConfig, getRingColor } from '../../utils/utils'; +import { + getStatusConfig, + getRingColor, + getYOffsetForCenterLabel, + getHeightForCenterLabel, +} from '../../utils/utils'; import CustomLegend from '../Scorecard/CustomLegend'; -import { CustomTooltip } from './CustomTooltip'; +import { ErrorTooltip } from '../Common/ErrorTooltip'; import { ResponsivePieChart } from './ResponsivePieChart'; const CenterLabel = ({ @@ -30,11 +35,15 @@ const CenterLabel = ({ cy, label, color, + onLabelMouseEnter, + onLabelMouseLeave, }: { cx: number; cy: number; label: string; color?: string; + onLabelMouseEnter?: (e: React.MouseEvent) => void; + onLabelMouseLeave?: (e: React.MouseEvent) => void; }) => { const fontSize = 14; const lineHeight = 1.2; @@ -42,28 +51,6 @@ const CenterLabel = ({ const textRef = useRef(null); const [layout, setLayout] = useState({ yOffset: -10, height: 40 }); - const getYOffset = (lineCount: number) => { - switch (lineCount) { - case 2: - return -17; - case 3: - return -24; - default: - return -8; - } - }; - - const getHeight = (lineCount: number) => { - switch (lineCount) { - case 2: - return 48; - case 3: - return 56; - default: - return 40; - } - }; - useLayoutEffect(() => { const el = textRef.current; if (!el) return; @@ -71,8 +58,8 @@ const CenterLabel = ({ const lineHeightPx = fontSize * lineHeight; const lineCount = Math.round(el.scrollHeight / lineHeightPx); - const nextOffset = getYOffset(lineCount); - const nextHeight = getHeight(lineCount); + const nextOffset = getYOffsetForCenterLabel(lineCount); + const nextHeight = getHeightForCenterLabel(lineCount); setLayout(prev => prev.yOffset === nextOffset && prev.height === nextHeight @@ -99,7 +86,10 @@ const CenterLabel = ({ textAlign: 'center', lineHeight, wordBreak: 'break-word', + cursor: 'pointer', }} + onMouseEnter={onLabelMouseEnter} + onMouseLeave={onLabelMouseLeave} > {label} @@ -120,6 +110,9 @@ export const EmptyStatePanel = ({ const theme = useTheme(); const { t } = useTranslation(); + const [isLabelHovered, setIsLabelHovered] = useState(false); + const [isInsidePieCircle, setIsInsidePieCircle] = useState(false); + const titleKey = `metric.${metricId}.title`; const descriptionKey = `metric.${metricId}.description`; @@ -194,20 +187,33 @@ export const EmptyStatePanel = ({ cy={centerY} label={label} color={color} + onLabelMouseEnter={e => { + setIsLabelHovered(true); + e.stopPropagation(); + }} + onLabelMouseLeave={e => { + setIsLabelHovered(false); + e.stopPropagation(); + }} /> ); }} legendContent={props => ( )} - tooltipContent={props => ( - - )} + tooltipContent={({ coordinate }) => { + const showTooltip = isLabelHovered || isInsidePieCircle; + + if (!showTooltip || coordinate === undefined) return null; + return ( + + ); + }} + isErrorState + setIsInsidePieCircle={setIsInsidePieCircle} /> diff --git a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ResponsivePieChart.tsx b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ResponsivePieChart.tsx index 01a446b043..a630a1c855 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ResponsivePieChart.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ResponsivePieChart.tsx @@ -46,6 +46,7 @@ export interface PieLegendContentProps { interface PieTooltipContentProps { active?: boolean; + coordinate?: { x: number; y: number }; payload?: readonly PieTooltipPayload[]; label?: string | number; } @@ -54,6 +55,8 @@ interface ResponsivePieChartProps { LabelContent?: (props: PieLabelRenderProps) => React.ReactNode; legendContent: (props: PieLegendContentProps) => React.ReactNode; tooltipContent: (props: PieTooltipContentProps) => React.ReactNode; + isErrorState?: boolean; + setIsInsidePieCircle?: (isInside: boolean) => void; } export const ResponsivePieChart = ({ @@ -61,10 +64,33 @@ export const ResponsivePieChart = ({ LabelContent, legendContent, tooltipContent, + isErrorState, + setIsInsidePieCircle, }: ResponsivePieChartProps) => { return ( + {/* This is the circle that is used to trigger the tooltip */} + {isErrorState && ( + + { + setIsInsidePieCircle?.(true); + e.stopPropagation?.(); + }} + onMouseLeave={e => { + setIsInsidePieCircle?.(false); + e.stopPropagation(); + }} + /> + + )} + { + setIsInsidePieCircle?.(true); + }} + onMouseLeave={() => { + setIsInsidePieCircle?.(false); + }} > {pieData.map(category => ( diff --git a/workspaces/scorecard/plugins/scorecard/src/utils/utils.ts b/workspaces/scorecard/plugins/scorecard/src/utils/utils.ts index 59bf84ef68..299aef3651 100644 --- a/workspaces/scorecard/plugins/scorecard/src/utils/utils.ts +++ b/workspaces/scorecard/plugins/scorecard/src/utils/utils.ts @@ -70,3 +70,25 @@ export const getRingColor = ( const [paletteKey, shade] = statusColor.split('.'); return theme.palette?.[paletteKey]?.[shade] ?? statusColor; }; + +export const getYOffsetForCenterLabel = (lineCount: number) => { + switch (lineCount) { + case 2: + return -17; + case 3: + return -24; + default: + return -8; + } +}; + +export const getHeightForCenterLabel = (lineCount: number) => { + switch (lineCount) { + case 2: + return 48; + case 3: + return 56; + default: + return 40; + } +};