Skip to content

Fixes #39317 - Update icons from pf3 to pf5#790

Merged
Lukshio merged 1 commit into
theforeman:masterfrom
pondrejk:pf5icons
May 19, 2026
Merged

Fixes #39317 - Update icons from pf3 to pf5#790
Lukshio merged 1 commit into
theforeman:masterfrom
pondrejk:pf5icons

Conversation

@pondrejk

Copy link
Copy Markdown
Contributor

Overview of Changes

  • PF5 icon

@Lukshio Lukshio left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace icon props to PF5, typedoes not exists and className is replaced by iconSize

@pondrejk

Copy link
Copy Markdown
Contributor Author

I'd leave this one for now as is, it points to the emptystate from foreman, which I guess we'll need to update anyway after theforeman/foreman#10986 gets in

@Lukshio

Lukshio commented May 18, 2026

Copy link
Copy Markdown

I'd leave this one for now as is, it points to the emptystate from foreman, which I guess we'll need to update anyway after theforeman/foreman#10986 gets in

EmptyState accepts icon component as a prop. So it should be replaced.

@pondrejk

Copy link
Copy Markdown
Contributor Author

EmptyState accepts icon component as a prop. So it should be replaced.

I got rid of the iconType prop, but from what I could find on the foreman side the icon="lock" prop should work ok as EmptyState runs it trough the iconMapper https://github.com/pondrejk/foreman/blob/60bc5fd267080873cb5db30ca227f4c1d713d44f/webpack/assets/javascripts/react_app/components/common/Icon/IconMapper.js#L78

@Lukshio

Lukshio commented May 18, 2026

Copy link
Copy Markdown

I got rid of the iconType prop, but from what I could find on the foreman side the icon="lock" prop should work ok as EmptyState runs it trough the iconMapper https://github.com/pondrejk/foreman/blob/60bc5fd267080873cb5db30ca227f4c1d713d44f/webpack/assets/javascripts/react_app/components/common/Icon/IconMapper.js#L78

Changes looks good. I think that the EmptyState should be backwards compatible after update. Please update the test snapshot. Packit tested.

@pondrejk

Copy link
Copy Markdown
Contributor Author

...Please update the test snapshot.

Updated, thanks!

@Lukshio Lukshio left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good. Thanks @pondrejk

@Lukshio Lukshio merged commit 4a33b18 into theforeman:master May 19, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants