Skip to content

Commit 9462602

Browse files
authored
ECOPROJECT-4632 | fix: solving color inconsistency between dark and light mode (#763)
- Implemented theme-aware styling for popovers, tooltips, and chart flyouts across the app. This restores and extends a previous fix that was lost after the PatternFly v6.5 upgrade. - Added a shared utility at src/lib/patternfly/flyoutAppendTo.ts that provides: 1. Theme-aware CSS for Popover and Tooltip surfaces (floating background + regular text) 2. Themed chart tooltip styles for Victory ChartTooltip flyouts 3. appendTo={document.body} so flyouts inherit PatternFly typography --------- Signed-off-by: Montse Ortega <mortegag@redhat.com>
1 parent ff89446 commit 9462602

21 files changed

Lines changed: 234 additions & 50 deletions
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { getFlyoutAppendTo } from "../flyoutAppendTo";
4+
5+
describe("getFlyoutAppendTo", () => {
6+
it("appends flyouts to document.body for PatternFly typography inheritance", () => {
7+
expect(getFlyoutAppendTo()).toBe(document.body);
8+
});
9+
});
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { css, cx } from "@emotion/css";
2+
3+
/**
4+
* DOM target for portaled PatternFly flyouts (Popover, Tooltip).
5+
* Uses `document.body` so flyouts inherit Red Hat Text from PatternFly base styles.
6+
* Appending to `document.documentElement` (where the theme class often lives) leaves
7+
* flyouts outside `body` and they fall back to the browser serif default.
8+
*/
9+
export const getFlyoutAppendTo = (): HTMLElement => document.body;
10+
11+
/** Shared typography for portaled popover and tooltip content. */
12+
export const flyoutTypographyClassName = css`
13+
.pf-v6-c-popover__content,
14+
.pf-v6-c-popover__title-text,
15+
.pf-v6-c-tooltip__content {
16+
font-family: var(--pf-t--global--font--family--body);
17+
line-height: var(--pf-t--global--font--line-height--body);
18+
}
19+
20+
.pf-v6-c-tooltip__content {
21+
font-size: var(--pf-t--global--font--size--body--sm);
22+
}
23+
`;
24+
25+
/** Popover color tokens that respect the active PatternFly theme. */
26+
export const themeAwarePopoverClassName = css`
27+
--pf-v6-c-popover__content--BackgroundColor: var(
28+
--pf-t--global--background--color--floating--default
29+
);
30+
--pf-v6-c-popover__content--Color: var(--pf-t--global--text--color--regular);
31+
--pf-v6-c-popover__arrow--BackgroundColor: var(
32+
--pf-t--global--background--color--floating--default
33+
);
34+
--pf-v6-c-popover__arrow--BorderColor: var(
35+
--pf-t--global--border--color--default
36+
);
37+
`;
38+
39+
/** Tooltip color tokens that respect the active PatternFly theme. */
40+
export const themeAwareTooltipClassName = css`
41+
--pf-v6-c-tooltip__content--BackgroundColor: var(
42+
--pf-t--global--background--color--floating--default
43+
);
44+
--pf-v6-c-tooltip__content--Color: var(--pf-t--global--text--color--regular);
45+
--pf-v6-c-tooltip__arrow--BackgroundColor: var(
46+
--pf-t--global--background--color--floating--default
47+
);
48+
--pf-v6-c-tooltip__arrow--BorderColor: var(
49+
--pf-t--global--border--color--default
50+
);
51+
`;
52+
53+
/** Typography + theme tokens for popovers. */
54+
export const themePopoverFlyoutClassName = cx(
55+
flyoutTypographyClassName,
56+
themeAwarePopoverClassName,
57+
);
58+
59+
/** Typography + theme tokens for tooltips. */
60+
export const themeTooltipFlyoutClassName = cx(
61+
flyoutTypographyClassName,
62+
themeAwareTooltipClassName,
63+
);
64+
65+
/** Victory chart tooltips (Voronoi) that respect the active PatternFly theme. */
66+
export const themedChartTooltipStyle = {
67+
fontSize: 9,
68+
fill: "var(--pf-t--global--text--color--regular)",
69+
} as const;
70+
71+
export const themedChartTooltipFlyoutStyle = {
72+
stroke: "var(--pf-t--global--border--color--default)",
73+
strokeWidth: 1,
74+
fill: "var(--pf-t--global--background--color--floating--default)",
75+
} as const;
76+
77+
export const themedChartTooltipFlyoutPadding = {
78+
top: 6,
79+
bottom: 6,
80+
left: 10,
81+
right: 10,
82+
} as const;
83+
84+
/** Shared props for theme-aware PatternFly Popovers. */
85+
export const themePopoverFlyoutProps = {
86+
appendTo: getFlyoutAppendTo,
87+
className: themePopoverFlyoutClassName,
88+
} as const;
89+
90+
/** Shared props for theme-aware PatternFly Tooltips. */
91+
export const themeTooltipFlyoutProps = {
92+
appendTo: getFlyoutAppendTo,
93+
className: themeTooltipFlyoutClassName,
94+
} as const;

src/ui/assessment/views/AssessmentsTable.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import React, { useMemo, useState } from "react";
2929
import { Link, useNavigate } from "react-router-dom";
3030

3131
import type { AssessmentModel } from "../../../models/AssessmentModel";
32+
import { themeTooltipFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
3233
import { routes } from "../../../routing/Routes";
3334
import { EmptySearchResults } from "../../core/components/EmptySearchResults";
3435

@@ -521,12 +522,13 @@ export const AssessmentsTable: React.FC<AssessmentsTableProps> = ({
521522
{isColumnVisible("Name") && (
522523
<Td dataLabel={Columns.Name} modifier="truncate">
523524
{row.hasData ? (
524-
<Tooltip content={row.name}>
525+
<Tooltip {...themeTooltipFlyoutProps} content={row.name}>
525526
<Link to={routes.assessmentById(row.id)}>{row.name}</Link>
526527
</Tooltip>
527528
) : (
528529
<span>
529530
<Tooltip
531+
{...themeTooltipFlyoutProps}
530532
content={
531533
row.sourceType.toLowerCase().includes("rvtools")
532534
? "No inventory data found. The uploaded file may be corrupted. Please verify and re-upload."
@@ -537,7 +539,7 @@ export const AssessmentsTable: React.FC<AssessmentsTableProps> = ({
537539
<RhUiWarningFillIcon />
538540
</Icon>
539541
</Tooltip>{" "}
540-
<Tooltip content={row.name}>
542+
<Tooltip {...themeTooltipFlyoutProps} content={row.name}>
541543
<span>{row.name}</span>
542544
</Tooltip>
543545
</span>
@@ -592,6 +594,7 @@ export const AssessmentsTable: React.FC<AssessmentsTableProps> = ({
592594
<Td dataLabel={Columns.AssessmentReport}>
593595
<TableText>
594596
<Tooltip
597+
{...themeTooltipFlyoutProps}
595598
content={
596599
row.hasData
597600
? "View assessment report"

src/ui/core/components/MigrationChart.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ import { chart_color_blue_300 } from "@patternfly/react-tokens/dist/esm/chart_co
1313
import { chart_color_red_orange_400 } from "@patternfly/react-tokens/dist/esm/chart_color_red_orange_400";
1414
import React, { useMemo } from "react";
1515

16+
import {
17+
getFlyoutAppendTo,
18+
themePopoverFlyoutClassName,
19+
} from "../../../lib/patternfly/flyoutAppendTo";
20+
1621
interface OSData {
1722
name: string;
1823
count: number;
@@ -48,11 +53,9 @@ type DLength =
4853
// Styles
4954
// ---------------------------------------------------------------------------
5055

51-
const upgradeRecommendationPopover = css`
52-
.popover-override .pf-v5-c-popover__close .pf-v5-c-button.pf-m-plain {
53-
color: var(--pf-t--global--text--color--regular);
54-
}
55-
.popover-override .pf-v5-c-popover__close .pf-v5-c-button.pf-m-plain:hover {
56+
const upgradeRecommendationPopoverCloseButton = css`
57+
.pf-v6-c-popover__close .pf-v6-c-button.pf-m-plain,
58+
.pf-v6-c-popover__close .pf-v6-c-button.pf-m-plain:hover {
5659
color: var(--pf-t--global--text--color--regular);
5760
}
5861
`;
@@ -178,7 +181,8 @@ const MigrationChart: React.FC<MigrationChartProps> = ({
178181
{item.infoText ? (
179182
<FlexItem shrink={{ default: "shrink" }}>
180183
<Popover
181-
className={upgradeRecommendationPopover}
184+
appendTo={getFlyoutAppendTo}
185+
className={`${themePopoverFlyoutClassName} ${upgradeRecommendationPopoverCloseButton}`}
182186
position="bottom"
183187
headerContent="Upgrade to get support"
184188
bodyContent={<div>{item.infoText}</div>}

src/ui/core/components/MigrationDonutChart.tsx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
1-
import { ChartDonut, ChartLabel, ChartLegend } from "@patternfly/react-charts";
1+
import {
2+
ChartDonut,
3+
ChartLabel,
4+
ChartLegend,
5+
ChartTooltip,
6+
} from "@patternfly/react-charts";
27
import { Flex, FlexItem } from "@patternfly/react-core";
38
import React, { useCallback, useMemo } from "react";
49

10+
import {
11+
themedChartTooltipFlyoutPadding,
12+
themedChartTooltipFlyoutStyle,
13+
themedChartTooltipStyle,
14+
} from "../../../lib/patternfly/flyoutAppendTo";
515
import { CardEmptyState } from "./CardEmptyState";
616

717
interface OSData {
@@ -156,6 +166,17 @@ const MigrationDonutChart: React.FC<MigrationDonutChartProps> = ({
156166
return chartData.reduce((sum, item) => sum + (Number(item.y) || 0), 0);
157167
}, [chartData]);
158168

169+
const donutTooltip = useMemo(
170+
() => (
171+
<ChartTooltip
172+
style={themedChartTooltipStyle}
173+
flyoutStyle={themedChartTooltipFlyoutStyle}
174+
flyoutPadding={themedChartTooltipFlyoutPadding}
175+
/>
176+
),
177+
[],
178+
);
179+
159180
if (!data || data.length === 0) {
160181
return (
161182
<CardEmptyState
@@ -174,6 +195,7 @@ const MigrationDonutChart: React.FC<MigrationDonutChartProps> = ({
174195
<ChartDonut
175196
ariaDesc="Migration data donut chart"
176197
data={chartData}
198+
labelComponent={donutTooltip}
177199
labels={({ datum }) => {
178200
const safeDatum = datum as ChartDatum;
179201
const percent =

src/ui/environment/views/AgentStatusView.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { t_global_icon_color_status_info_default as globalInfoColor100 } from "@
2121
import React, { useMemo } from "react";
2222
import { Link } from "react-router-dom";
2323

24+
import { themePopoverFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
2425
import { VCenterSetupInstructions } from "../../core/components/VCenterSetupInstructions";
2526
import { safeExternalUrl } from "../../core/utils/urlValidation";
2627
import { getDiscoveryVmStatusLabel } from "../helpers/discoveryVmStatus";
@@ -156,6 +157,7 @@ export const AgentStatusView: React.FC<AgentStatusView.Props> = (props) => {
156157
status !== "not-connected" &&
157158
status !== "up-to-date") ? (
158159
<Popover
160+
{...themePopoverFlyoutProps}
159161
aria-label={statusView && statusView.text}
160162
headerContent={statusView && statusView.text}
161163
headerComponent="h1"
@@ -186,6 +188,7 @@ export const AgentStatusView: React.FC<AgentStatusView.Props> = (props) => {
186188
{isNotConnected && (
187189
<SplitItem>
188190
<Popover
191+
{...themePopoverFlyoutProps}
189192
aria-label="Setup instructions"
190193
headerContent="Setup instructions"
191194
headerComponent="h2"

src/ui/environment/views/DownloadOvaAction.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Button, Icon, Tooltip } from "@patternfly/react-core";
22
import { RhUiDownloadIcon } from "@patternfly/react-icons";
33
import React, { useCallback, useState } from "react";
44

5+
import { themeTooltipFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
56
import { useEnvironmentPage } from "../view-models/EnvironmentPageContext";
67

78
// eslint-disable-next-line @typescript-eslint/no-namespace
@@ -40,7 +41,7 @@ export const DownloadOvaAction: React.FC<DownloadOvaAction.Props> = (props) => {
4041
}, [sourceId, sourceName, url]);
4142

4243
return (
43-
<Tooltip content="Download OVA File">
44+
<Tooltip {...themeTooltipFlyoutProps} content="Download OVA File">
4445
<Button
4546
icon={
4647
<Icon size="md" isInline>

src/ui/environment/views/ProxyFields.tsx

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
import { RhUiQuestionMarkCircleIcon } from "@patternfly/react-icons";
1313
import React, { useState } from "react";
1414

15+
import { themePopoverFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
16+
1517
const ProxyInputFields = ({
1618
httpProxy,
1719
httpsProxy,
@@ -29,7 +31,10 @@ const ProxyInputFields = ({
2931
<FormGroup
3032
label="HTTP proxy URL"
3133
labelHelp={
32-
<Popover bodyContent="The HTTP proxy URL that agents should use to access the discovery service.">
34+
<Popover
35+
{...themePopoverFlyoutProps}
36+
bodyContent="The HTTP proxy URL that agents should use to access the discovery service."
37+
>
3338
<button
3439
type="button"
3540
aria-label="More info about HTTP proxy URL"
@@ -60,7 +65,10 @@ const ProxyInputFields = ({
6065
<FormGroup
6166
label="HTTPS proxy URL"
6267
labelHelp={
63-
<Popover bodyContent="Specify the HTTPS proxy that agents should use to access the discovery service. If you don't provide a value, your HTTP proxy URL will be used by default for both HTTP and HTTPS connections.">
68+
<Popover
69+
{...themePopoverFlyoutProps}
70+
bodyContent="Specify the HTTPS proxy that agents should use to access the discovery service. If you don't provide a value, your HTTP proxy URL will be used by default for both HTTP and HTTPS connections."
71+
>
6472
<button
6573
type="button"
6674
aria-label="More info about HTTPS proxy URL"
@@ -91,7 +99,10 @@ const ProxyInputFields = ({
9199
<FormGroup
92100
label="No proxy domains"
93101
labelHelp={
94-
<Popover bodyContent="Exclude destination domain names, IP addresses, or other network CIDRs from proxying by adding them to this comma-separated list.">
102+
<Popover
103+
{...themePopoverFlyoutProps}
104+
bodyContent="Exclude destination domain names, IP addresses, or other network CIDRs from proxying by adding them to this comma-separated list."
105+
>
95106
<button
96107
type="button"
97108
aria-label="More info about no proxy domains"

src/ui/environment/views/RemoveSourceAction.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Button, Content, Icon, Tooltip } from "@patternfly/react-core";
22
import { RhUiTrashIcon } from "@patternfly/react-icons";
33
import React, { useCallback, useState } from "react";
44

5+
import { themeTooltipFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
56
import { ConfirmationModal } from "../../core/components/ConfirmationModal";
67

78
// eslint-disable-next-line @typescript-eslint/no-namespace
@@ -46,7 +47,7 @@ export const RemoveSourceAction: React.FC<RemoveSourceAction.Props> = (
4647

4748
return (
4849
<>
49-
<Tooltip content="Remove">
50+
<Tooltip {...themeTooltipFlyoutProps} content="Remove">
5051
<Button
5152
icon={
5253
<Icon size="md" isInline>

src/ui/environment/views/SourcesTable.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import React, { useEffect, useMemo, useState } from "react";
2626
import { useNavigate } from "react-router-dom";
2727

2828
import type { SourceModel } from "../../../models/SourceModel";
29+
import { themeTooltipFlyoutProps } from "../../../lib/patternfly/flyoutAppendTo";
2930
import { routes } from "../../../routing/Routes";
3031
import { ConfirmationModal } from "../../core/components/ConfirmationModal";
3132
import { EmptySearchResults } from "../../core/components/EmptySearchResults";
@@ -721,6 +722,7 @@ export const SourcesTable: React.FC<SourceTableProps> = ({
721722
<Td dataLabel={Columns.LastSeen} className={tableCellTop}>
722723
{source?.updatedAt ? (
723724
<Tooltip
725+
{...themeTooltipFlyoutProps}
724726
content={new Date(source.updatedAt).toLocaleString()}
725727
>
726728
<span>{formatRelativeTime(source.updatedAt)}</span>

0 commit comments

Comments
 (0)