Skip to content

Migrate missed PF5 styles to PF6 (#10175)#252

Open
keithchong wants to merge 1 commit into
redhat-developer:mainfrom
keithchong:10175-PF6-Styles
Open

Migrate missed PF5 styles to PF6 (#10175)#252
keithchong wants to merge 1 commit into
redhat-developer:mainfrom
keithchong:10175-PF6-Styles

Conversation

@keithchong

Copy link
Copy Markdown
Collaborator

See GITOPS-10175

@openshift-ci openshift-ci Bot requested a review from wtam2018 June 29, 2026 18:37
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@keithchong, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 63620038-4159-4daf-a849-e16659ecd2d2

📥 Commits

Reviewing files that changed from the base of the PR and between 7a0dd27 and 06951b0.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • src/gitops/Statuses/HealthStatus.test.tsx
  • src/gitops/Statuses/OperationState.test.tsx
  • src/gitops/Statuses/SyncStatus.test.tsx
  • src/gitops/components/application/graph/ApplicationGraphView.scss
  • src/gitops/utils/components/Icons/Icons.tsx
📝 Walkthrough

Walkthrough

Migrates icon color CSS variable references from PatternFly v5 (--pf-v5-global--*) to PatternFly 6 (--pf-t--global--icon--color--*) tokens in exported color constants, SCSS step-edge styles, and corresponding test inline snapshots.

Changes

PatternFly v6 CSS Token Migration

Layer / File(s) Summary
Icon color constants
src/gitops/utils/components/Icons/Icons.tsx
Six exported color constants (dangerColor, blueDefaultColor, disabledColor, blueInfoColor, warningColor, successColor) are updated to reference PF6 --pf-t--global--icon--color--* CSS variables.
SCSS step-edge colors
src/gitops/components/application/graph/ApplicationGraphView.scss
stroke for .step-edge-healthy and .step-edge-warning, and both stroke and fill for .step-edge-terminal healthy/warning variants, are switched to the new PF6 status icon color tokens.
Test snapshot updates
src/gitops/Statuses/HealthStatus.test.tsx, src/gitops/Statuses/OperationState.test.tsx, src/gitops/Statuses/SyncStatus.test.tsx
Inline snapshot expectations for all status icon renders (Healthy, Degraded, Progressing, Unknown, popover, sync phases, quiet mode) are updated to match the new CSS variable names in rendered SVG style attributes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is related to the change, but it is too vague to convey meaningful details. Add a brief summary of the PF5-to-PF6 style migration and the affected areas.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: migrating remaining PF5 styles to PF6.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 11.80%. Comparing base (01e9db1) to head (06951b0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
- Coverage   11.92%   11.80%   -0.12%     
==========================================
  Files         154      154              
  Lines        6272     6344      +72     
  Branches     2028     2172     +144     
==========================================
+ Hits          748      749       +1     
+ Misses       5524     5368     -156     
- Partials        0      227     +227     
Flag Coverage Δ
unit-tests 11.80% <100.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/gitops/Statuses/HealthStatus.test.tsx`:
- Around line 11-33: The HealthStatus test file uses JSX in the snapshots and
render calls but is missing the React import required by the current
react/react-in-jsx-scope lint rule. Add the React import at the top of
HealthStatus.test.tsx so the JSX in the HealthStatus test cases compiles and
lints correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 0b335a72-6586-4d90-a02a-d744db6179ae

📥 Commits

Reviewing files that changed from the base of the PR and between 01e9db1 and 7a0dd27.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • src/gitops/Statuses/HealthStatus.test.tsx
  • src/gitops/Statuses/OperationState.test.tsx
  • src/gitops/Statuses/SyncStatus.test.tsx
  • src/gitops/components/application/graph/ApplicationGraphView.scss
  • src/gitops/utils/components/Icons/Icons.tsx

Comment on lines 11 to +33
it('renders Degraded', () => {
expect(renderToStaticMarkup(<HealthStatus status="Degraded" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="HeartBrokenIcon" style="color:var(--pf-v5-global--danger-color--100)"></svg> Degraded</div></div>"`,
`"<div><div><svg data-icon="HeartBrokenIcon" style="color:var(--pf-t--global--icon--color--status--danger--default)"></svg> Degraded</div></div>"`,
);
});

it('renders Progressing', () => {
expect(renderToStaticMarkup(<HealthStatus status="Progressing" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="CircleNotchIcon" class="undefined fa-spin" style="color:var(--pf-v5-global--primary-color--200)"></svg> Progressing</div></div>"`,
`"<div><div><svg data-icon="CircleNotchIcon" class="undefined fa-spin" style="color:var(--pf-t--global--icon--color--brand--default)"></svg> Progressing</div></div>"`,
);
});

it('renders Unknown for unrecognized status', () => {
expect(renderToStaticMarkup(<HealthStatus status="SomethingElse" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="UnknownIcon" style="color:var(--pf-v5-global--disabled-color--100)"></svg> SomethingElse</div></div>"`,
`"<div><div><svg data-icon="UnknownIcon" style="color:var(--pf-t--global--icon--color--disabled)"></svg> SomethingElse</div></div>"`,
);
});

it('renders popover when message is provided', () => {
expect(
renderToStaticMarkup(<HealthStatus status="Degraded" message="Something broke" />),
).toMatchInlineSnapshot(
`"<div><div><div data-testid="popover"><div data-testid="popover-header"><div>Degraded</div></div><div data-testid="popover-body"><div>Something broke</div></div><button data-variant="link"><svg data-icon="HeartBrokenIcon" style="color:var(--pf-v5-global--danger-color--100)"></svg> Degraded</button></div></div></div>"`,
`"<div><div><div data-testid="popover"><div data-testid="popover-header"><div>Degraded</div></div><div data-testid="popover-body"><div>Something broke</div></div><button data-variant="link"><svg data-icon="HeartBrokenIcon" style="color:var(--pf-t--global--icon--color--status--danger--default)"></svg> Degraded</button></div></div></div>"`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '## file\n'
git ls-files 'src/gitops/Statuses/HealthStatus.test.tsx' 'src/gitops/Statuses/HealthStatus.tsx' '.eslintrc*' 'eslint.config.*' 'package.json' | sed 's#^`#-` #'

printf '\n## file excerpts\n'
for f in src/gitops/Statuses/HealthStatus.test.tsx src/gitops/Statuses/HealthStatus.tsx; do
  if [ -f "$f" ]; then
    echo "--- $f"
    nl -ba "$f" | sed -n '1,120p'
  fi
done

printf '\n## eslint/react config references\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
  'react/react-in-jsx-scope|react/jsx-runtime|jsx-runtime|jsxImportSource|`@typescript-eslint`|eslint-plugin-react|react' \
  .eslintrc* eslint.config.* package.json . 2>/dev/null | sed -n '1,220p'

Repository: redhat-developer/gitops-console-plugin

Length of output: 404


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '## nearby test files with JSX imports\n'
rg -n --glob 'src/**/*.test.tsx' --glob 'src/**/*.tsx' \
  'import React from '\''react'\''|react/react-in-jsx-scope|jsx-runtime' src | sed -n '1,240p'

Repository: redhat-developer/gitops-console-plugin

Length of output: 372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## src/gitops/Statuses/HealthStatus.test.tsx"
sed -n '1,120p' src/gitops/Statuses/HealthStatus.test.tsx

echo
echo "## .eslintrc.json"
sed -n '1,220p' .eslintrc.json

echo
echo "## package.json relevant bits"
sed -n '1,220p' package.json | sed -n '/"eslint"/,/}/p'

echo
echo "## React imports in nearby tsx files"
rg -n --glob 'src/**/*.tsx' "import React from 'react';|react/react-in-jsx-scope|jsx-runtime" src | sed -n '1,240p'

Repository: redhat-developer/gitops-console-plugin

Length of output: 7008


Fix the JSX-in-scope lint failure in this test file.
src/gitops/Statuses/HealthStatus.test.tsx uses JSX but doesn't import React, so react/react-in-jsx-scope fails under the current ESLint config.

Suggested fix
+import React from 'react';
 import { renderToStaticMarkup } from 'react-dom/server';
 import HealthStatus, { HealthStatusIcon } from './HealthStatus';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('renders Degraded', () => {
expect(renderToStaticMarkup(<HealthStatus status="Degraded" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="HeartBrokenIcon" style="color:var(--pf-v5-global--danger-color--100)"></svg> Degraded</div></div>"`,
`"<div><div><svg data-icon="HeartBrokenIcon" style="color:var(--pf-t--global--icon--color--status--danger--default)"></svg> Degraded</div></div>"`,
);
});
it('renders Progressing', () => {
expect(renderToStaticMarkup(<HealthStatus status="Progressing" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="CircleNotchIcon" class="undefined fa-spin" style="color:var(--pf-v5-global--primary-color--200)"></svg> Progressing</div></div>"`,
`"<div><div><svg data-icon="CircleNotchIcon" class="undefined fa-spin" style="color:var(--pf-t--global--icon--color--brand--default)"></svg> Progressing</div></div>"`,
);
});
it('renders Unknown for unrecognized status', () => {
expect(renderToStaticMarkup(<HealthStatus status="SomethingElse" />)).toMatchInlineSnapshot(
`"<div><div><svg data-icon="UnknownIcon" style="color:var(--pf-v5-global--disabled-color--100)"></svg> SomethingElse</div></div>"`,
`"<div><div><svg data-icon="UnknownIcon" style="color:var(--pf-t--global--icon--color--disabled)"></svg> SomethingElse</div></div>"`,
);
});
it('renders popover when message is provided', () => {
expect(
renderToStaticMarkup(<HealthStatus status="Degraded" message="Something broke" />),
).toMatchInlineSnapshot(
`"<div><div><div data-testid="popover"><div data-testid="popover-header"><div>Degraded</div></div><div data-testid="popover-body"><div>Something broke</div></div><button data-variant="link"><svg data-icon="HeartBrokenIcon" style="color:var(--pf-v5-global--danger-color--100)"></svg> Degraded</button></div></div></div>"`,
`"<div><div><div data-testid="popover"><div data-testid="popover-header"><div>Degraded</div></div><div data-testid="popover-body"><div>Something broke</div></div><button data-variant="link"><svg data-icon="HeartBrokenIcon" style="color:var(--pf-t--global--icon--color--status--danger--default)"></svg> Degraded</button></div></div></div>"`,
import React from 'react';
import { renderToStaticMarkup } from 'react-dom/server';
import HealthStatus, { HealthStatusIcon } from './HealthStatus';
🧰 Tools
🪛 ESLint

[error] 12-12: 'React' must be in scope when using JSX

(react/react-in-jsx-scope)


[error] 18-18: 'React' must be in scope when using JSX

(react/react-in-jsx-scope)


[error] 24-24: 'React' must be in scope when using JSX

(react/react-in-jsx-scope)


[error] 31-31: 'React' must be in scope when using JSX

(react/react-in-jsx-scope)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/gitops/Statuses/HealthStatus.test.tsx` around lines 11 - 33, The
HealthStatus test file uses JSX in the snapshots and render calls but is missing
the React import required by the current react/react-in-jsx-scope lint rule. Add
the React import at the top of HealthStatus.test.tsx so the JSX in the
HealthStatus test cases compiles and lints correctly.

Source: Linters/SAST tools

Signed-off-by: Keith Chong <kykchong@redhat.com>
@keithchong keithchong requested a review from aali309 June 29, 2026 19:39
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