CONSOLE-3955: Migrate LogSnippet to PatternFly component#16190
CONSOLE-3955: Migrate LogSnippet to PatternFly component#16190stefanonardo wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
@stefanonardo: This pull request references CONSOLE-3955 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@stefanonardo: This pull request references CONSOLE-3955 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThe ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/topology/src/components/workload/BuildOverview.tsx`:
- Line 2: The import is using the package internals for LogSnippet; change the
import to use the package root by replacing the internal path import of
LogSnippet with "import { LogSnippet } from
'@patternfly/react-component-groups'"; confirm that LogSnippet is exported from
`@patternfly/react-component-groups`@6.4.0 before changing (or if not exported,
add a short comment documenting the dependency and open an issue with PatternFly
to request a root export), and update any other occurrences in BuildOverview.tsx
that reference the internal path to keep imports consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 323e6a7f-4dd8-4a1e-a0c7-f7125009cc44
📒 Files selected for processing (6)
frontend/packages/console-shared/src/components/index.tsfrontend/packages/console-shared/src/components/log/LogSnippet.scssfrontend/packages/console-shared/src/components/log/LogSnippet.tsxfrontend/packages/console-shared/src/components/log/index.tsfrontend/packages/topology/src/components/workload/BuildOverview.scssfrontend/packages/topology/src/components/workload/BuildOverview.tsx
💤 Files with no reviewable changes (4)
- frontend/packages/console-shared/src/components/log/index.ts
- frontend/packages/console-shared/src/components/index.ts
- frontend/packages/console-shared/src/components/log/LogSnippet.scss
- frontend/packages/console-shared/src/components/log/LogSnippet.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/topology/src/components/workload/BuildOverview.scssfrontend/packages/topology/src/components/workload/BuildOverview.tsx
🔀 Multi-repo context openshift/console-operator
Linked repositories findings
openshift/console-operator
- vendor/github.com/openshift/api/build/v1/types.go — contains a Build API field
LogSnippet stringjson:"logSnippet,omitempty"`.[::openshift/console-operator::vendor/github.com/openshift/api/build/v1/types.go] - vendor/github.com/openshift/api/build/v1/generated.pb.go — multiple references to
LogSnippetin protobuf marshaling/unmarshaling and string output (e.g.,i -= len(m.LogSnippet),m.LogSnippet = string(...),`LogSnippet:` + fmt.Sprintf("%v", this.LogSnippet) +, etc.).[::openshift/console-operator::vendor/github.com/openshift/api/build/v1/generated.pb.go]
Notes: These show that the cluster Build API surfaces a logSnippet field (data model), which consumers (UIs) may render. The PR removed the LogSnippet React component from @console/shared and switched to a PatternFly component in topology; ensure other UI consumers that previously imported LogSnippet from @console/shared are updated to use the new PatternFly component or updated import paths.
🔇 Additional comments (2)
frontend/packages/topology/src/components/workload/BuildOverview.scss (1)
11-14: LGTM — CSS addition is consistent with existing patterns.The new class follows the BEM naming convention already established in this file. The padding values align the log snippet content appropriately within the PatternFly alert layout.
frontend/packages/topology/src/components/workload/BuildOverview.tsx (1)
87-93: Migration to PatternFly LogSnippet confirmed—clean migration with no lingering dependencies.Verification shows no imports of the old
LogSnippetfrom@console/sharedremain in the console repo. The component correctly imports from@patternfly/react-component-groups/src/LogSnippetand aligns with PatternFly's API.Minor note: if
messageisundefinedin the Build status, the code renders<p>undefined</p>. While acceptable, this could be tightened with a simple conditional likemessage && <p>{message}</p>to avoid the empty paragraph.
frontend/packages/topology/src/components/workload/BuildOverview.tsx
Outdated
Show resolved
Hide resolved
3ee31a8 to
fd4b2ec
Compare
|
@stefanonardo: This pull request references CONSOLE-3955 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
logonoff
left a comment
There was a problem hiding this comment.
Nice!!
/label px-approved
/label docs-approved
| return unsuccessful ? ( | ||
| <LogSnippet | ||
| className="build-overview__log-snippet" | ||
| message={<p>{message}</p>} |
There was a problem hiding this comment.
LogSnippet should already wrap the content in a pre or code so the p isn't necessary
There was a problem hiding this comment.
@logonoff we do this to suppress the alert icon since we already display it in the component above the LogSnippet: https://github.com/patternfly/react-component-groups/blob/main/packages/module/src/LogSnippet/LogSnippet.tsx#L19
keep in mind that message is the "title" of the log snippet, the actual log is passed in logSnippet
|
|
||
| .build-overview__log-snippet { | ||
| padding: 10px 0 10px 19px; | ||
| } |
There was a problem hiding this comment.
in general we prefer using PatternFly tokens and CSS where possible. there are a bunch of utility classes (https://www.patternfly.org/utility-classes/spacing) you can use which align with the PatternFly token system (https://www.patternfly.org/tokens/all-patternfly-tokens)
There was a problem hiding this comment.
updated, thank you for the tip
|
@stefanonardo: This pull request references CONSOLE-3955 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
fd4b2ec to
247c83f
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, stefanonardo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
@stefanonardo: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/assign @yapei |


Replace custom LogSnippet with the PatternFly component.
The PatternFly LogSnippet renders an alert by default instead of a left red border.
After:
Summary by CodeRabbit
Refactor
Style