Fixes #38879 - Update BarChart to Patternfly5 by tlabaj · Pull Request #10743 · theforeman/foreman

tlabaj · 2025-10-30T15:24:05Z

Fixes: #38879

This PR updates the Bar Chart to use the PF5 components.
API's have been maintained as to not introduce breaking changes to any plugins that may be using the component.
Tests have been updated to use RTL.

Before:

After:

adamruzicka · 2025-10-30T15:33:00Z

Looking at those screenshots, could the chart fill the entire width of the card like it used to?

kmalyjur

Thank you, I'm leaving some comments.
I haven't tested the chart in the foreman_openscap yet.

kmalyjur · 2025-11-26T12:06:48Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

 import React from 'react';
 import PropTypes from 'prop-types';
-import { BarChart as PfBarChart } from 'patternfly-react';
-import { getBarChartConfig } from '../../../../../services/charts/BarChartService';


The BarChartService.js file could be deleted when it's not used anymore

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

kmalyjur · 2025-11-26T12:31:29Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

-  return <MessageBox msg={noDataMsg} icontype="info" />;
+
+  // Configuration based on size
+  const getChartDimensions = () => {


As Adam noticed, it would be nice if it filled the space

I do not have data in my bar chart, but this is how it looks filling the space.

kmalyjur · 2025-11-26T15:27:58Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

This function could be moved outside of the BarChart component

kmalyjur · 2025-11-26T15:43:37Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

The custom coloring doesn't work

kmalyjur · 2025-11-26T15:54:47Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/BarChart.test.js

fireEvent is not used

kmalyjur · 2025-11-26T15:59:34Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/BarChart.test.js

'container' is declared but its value is never read.ts(6133)

The same happens for "renders with custom axis labels" test

lhellebr · 2025-12-02T15:40:40Z

@kmalyjur Is this ~finished enough to test with packit?

kmalyjur · 2025-12-03T09:57:15Z

@kmalyjur Is this ~finished enough to test with packit?

@lhellebr I'm not sure, to be honest. I would wait for the changes.

kmalyjur

Everything is great now, but I forgot that when deleting functions (getBarChartConfig) in foreman core, we need to add a deprecation message first. I'm sorry for not mentioning this before.

There is a deprecation function for that, here is an example of usage: https://github.com/theforeman/foreman/pull/8267/files.
The message could be something like "Please use COMPONENT_NAME from IMPORT_PATH instead".
Could you please not delete getBarChartConfig (and the tests) and deprecate it instead?

Also, the file foreman/config/irbrc_history could be deleted.

kmalyjur · 2026-01-02T14:06:55Z

Thank you, now it just needs to be verified

lhellebr · 2026-01-07T16:00:22Z

/packit build

lhellebr · 2026-01-07T16:03:46Z

https://copr.fedorainfracloud.org/coprs/fulltext/?fulltext=packit%2Ftheforeman-foreman-10743 is missing, can someone build it please?

adamruzicka · 2026-01-07T16:04:51Z

/packit build

lhellebr · 2026-01-08T14:36:15Z

FailedQA with foreman-3.18.0-0.20260107161624001555.pr10743.8932.g7a7c7f239.el9.noarch on Firefox 146.0 build 20251211125526 and Chromium 143.0.7499.169.

When hovered, the tooltip isn't shown so it's not at all clear from the graph what value exactly the bar represents.
Original state:

New state:

In SCAP, there is a table next to the graph, but that's not true for other places.

lhellebr · 2026-02-04T20:41:32Z

@tlabaj there have been 57 commits added to this PR, that doesn't seem right

tlabaj · 2026-02-09T22:19:13Z

@lhellebr @kmalyjur I updated the tooltip. I also fixed the commit issue.

kmalyjur

I tested this using bundle exec rake seed:reports origin=Puppet hosts=3 reports=5 lines=10, but I couldn't see the tooltips until I made these adjustments.
What do you think about it? If it works for you just fine without my changes, I might have something wrong with my environment. I'm honestly not sure about it.

kmalyjur · 2026-02-11T15:38:02Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

+  return rawData.map(item => ({
+    x: item[0], // label
+    y: item[1], // value
+    name: item[0],
+    color: item[2] || undefined, // custom color if provided
+  }));


Suggested change

return rawData.map(item => ({

x: item[0], // label

y: item[1], // value

name: item[0],

color: item[2] || undefined, // custom color if provided

}));

return rawData.map((item, index) => ({

x: item?.x ?? item[0], // label

y: item?.y ?? item[1], // value

name: item?.name ?? (item?.x ?? item[0]),

color: item?.color ?? (item[2] || undefined), // custom color if provided

}));

kmalyjur · 2026-02-11T15:44:27Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

+          <ChartVoronoiContainer
+            labels={({ datum }) => getTooltipValue(datum)}


This makes the hover area vertical slices, so it's easier to trigger the tooltip

Suggested change

<ChartVoronoiContainer

labels={({ datum }) => getTooltipValue(datum)}

<ChartVoronoiContainer

voronoiDimension="x"

labels={({ datum }) => getTooltipValue(datum)}

kmalyjur · 2026-02-11T15:46:53Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

+                constrainToVisibleArea
+                renderInPortal


Deleting renderInPortal

Suggested change

constrainToVisibleArea

renderInPortal

constrainToVisibleArea

kmalyjur · 2026-02-11T15:48:34Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

Suggested change

BarChart.propTypes = {

data: PropTypes.arrayOf(PropTypes.any),

kmalyjur · 2026-02-12T10:27:36Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

+            style={{
+              background: 'var(--pf-v5-global--BackgroundColor--dark-100)',
+              color: 'var(--pf-v5-global--Color--light-100)',
+              padding:
+                'var(--pf-v5-global--spacer--sm) var(--pf-v5-global--spacer--md)',
+              borderRadius: 'var(--pf-v5-global--BorderRadius--sm)',
+              fontSize: 'var(--pf-v5-global--FontSize--sm)',
+              fontFamily: 'var(--pf-v5-global--FontFamily--sans-serif)',
+            }}
+          >
+            <div style={{ fontWeight: 700 }}>{tooltipTitle}</div>
+            <Divider
+              style={{
+                margin: 'var(--pf-v5-global--spacer--xs) 0',
+                backgroundColor: 'var(--pf-v5-global--Color--200)',
+              }}
+            />
+            <div style={{ display: 'flex', alignItems: 'center' }}>
+              <span
+                style={{
+                  width: 'var(--pf-v5-global--spacer--sm)',
+                  height: 'var(--pf-v5-global--spacer--sm)',
+                  background: color,
+                  display: 'inline-block',
+                  marginRight: 'var(--pf-v5-global--spacer--xs)',
+                }}
+              />
+              <span>Value:</span>
+              <span style={{ marginLeft: 'var(--pf-v5-global--spacer--xs)' }}>


We should move the inline styles into a separate BarChart.scss.

kmalyjur · 2026-02-12T10:41:00Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

-        unloadBeforeLoad={unloadData}
-      />
+      <g>
+        <foreignObject


The tooltip looks great, however, can I please ask why foreignObject is used instead of ChartTooltip from PF?

I am using the ChartTooltip. I followed the PF example on how to embed HTML in the tooltip and foreignObject is used. This was the best way to get the tooltip to look like it was with the prior implementation.

That makes sense, thank you for the explanation

kmalyjur · 2026-02-12T11:09:02Z

webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js

+      x: PropTypes.any,
+      y: PropTypes.any,
+      name: PropTypes.any,


There could be more specific PropTypes used than any.

lhellebr · 2026-02-12T13:52:49Z

I, just as @kmalyjur , wasn't able to get the tooltips working out of the box => failed.

pr-processor bot added Not yet reviewed Waiting on contributor labels Oct 30, 2025

github-actions bot added the UI label Oct 30, 2025

tlabaj force-pushed the barchart branch from 1c0faa0 to 5982e73 Compare November 3, 2025 15:38

pr-processor bot removed the Waiting on contributor label Nov 3, 2025

tlabaj changed the title ~~Barchart~~ Fixes #38879 - Update BarChart to Patternfly5 Nov 3, 2025

lhellebr self-requested a review November 24, 2025 12:17

kmalyjur suggested changes Nov 26, 2025

View reviewed changes

pr-processor bot added Waiting on contributor and removed Not yet reviewed labels Nov 26, 2025

MariaAga added the breaks-robottelo label Dec 1, 2025

tlabaj force-pushed the barchart branch from 5982e73 to c15a6cc Compare December 8, 2025 20:25

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 8, 2025

tlabaj force-pushed the barchart branch from c15a6cc to ed73900 Compare December 8, 2025 20:31

pr-processor bot removed the Needs re-review label Dec 8, 2025

tlabaj force-pushed the barchart branch from ed73900 to 33a3ac6 Compare December 10, 2025 20:43

kmalyjur suggested changes Dec 15, 2025

View reviewed changes

pr-processor bot added the Waiting on contributor label Dec 15, 2025

tlabaj force-pushed the barchart branch from 33a3ac6 to d560c80 Compare December 17, 2025 15:29

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 17, 2025

tlabaj force-pushed the barchart branch from d560c80 to 7f8d048 Compare December 18, 2025 21:24

tlabaj requested a review from a team as a code owner February 4, 2026 20:27

github-actions bot added Templates Docs labels Feb 4, 2026

tlabaj force-pushed the barchart branch 3 times, most recently from 30d3a1e to 8e3e638 Compare February 9, 2026 22:14

Fixes #38879 - Update BarChart to Patternfly5

680a70a

tlabaj force-pushed the barchart branch from 8e3e638 to 680a70a Compare February 9, 2026 22:29

kmalyjur reviewed Feb 11, 2026

View reviewed changes

pr-processor bot removed the Needs re-review label Feb 11, 2026

kmalyjur suggested changes Feb 12, 2026

View reviewed changes

pr-processor bot added the Waiting on contributor label Feb 12, 2026

		<ChartVoronoiContainer
		labels={({ datum }) => getTooltipValue(datum)}


	BarChart.propTypes = {
	data: PropTypes.arrayOf(PropTypes.any),

Conversation

tlabaj commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamruzicka commented Oct 30, 2025

Uh oh!

kmalyjur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhellebr commented Dec 2, 2025

Uh oh!

kmalyjur commented Dec 3, 2025

Uh oh!

kmalyjur left a comment

Choose a reason for hiding this comment

Uh oh!

kmalyjur commented Jan 2, 2026

Uh oh!

lhellebr commented Jan 7, 2026

Uh oh!

lhellebr commented Jan 7, 2026

Uh oh!

adamruzicka commented Jan 7, 2026

Uh oh!

lhellebr commented Jan 8, 2026

Uh oh!

lhellebr commented Feb 4, 2026

Uh oh!

tlabaj commented Feb 9, 2026

Uh oh!

kmalyjur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmalyjur Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhellebr commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

tlabaj commented Oct 30, 2025 •

edited

Loading

kmalyjur Feb 12, 2026 •

edited

Loading