MGMT-21823: Review and create page styling not applied #3216
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jgyselov: This pull request references MGMT-21823 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 task to target the "4.21.0" version, but no target version was set. In 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. |
openshift-ci
bot
added
approved
WalkthroughReplaced a custom summary wrapper with PatternFly ExpandableSection, adjusted CSS selectors for the summary expandable toggle/icon, added a text-alignment utility to a GridItem in preflight checks, and wrapped toggle titles in PatternFly Content with bold utility. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ReviewSummary
participant ExpandableSection as "PatternFly ExpandableSection"
participant Content as "ReviewSummaryContent"
User->>ReviewSummary: open Review step
ReviewSummary->>ExpandableSection: render (id="summary-expandable", isExpanded=true, isIndented, toggleContent)
ExpandableSection->>Content: display summary content
note over ExpandableSection,Content: Toggle exists but content is forced expanded
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/TableSummaryExpandable.tsx (1)
16-16: Approve with note on styling inconsistency.Using PatternFly's
Contentcomponent with the utility class is the recommended approach for PF6. However, note thatReviewSummary.tsx(lines 72) uses plain text fortoggleContentinstead of wrapping it inContentlike this component does. Consider applying this same pattern inReviewSummary.tsxfor consistency:- toggleContent={'Cluster summary'} + toggleContent={<Content className="pf-v6-u-font-weight-bold">Cluster summary</Content>}libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewSummary.tsx (1)
71-78: Verify if always-expanded behavior is intentional.The
ExpandableSectionhasisExpanded={true}but noonTogglehandler, making it always expanded and non-interactive. If the intent is to keep the cluster summary always visible (as suggested by the "After" screenshot in the PR description), this is fine. However, if users should be able to collapse the summary, add anonTogglehandler:+const ReviewSummary = ({ cluster }: { cluster: Cluster }) => { + const [isExpanded, setIsExpanded] = React.useState(true); return ( <ExpandableSection toggleContent={'Cluster summary'} id={'summary-expandable'} - isExpanded={true} + isExpanded={isExpanded} + onToggle={() => setIsExpanded(!isExpanded)} isIndented > <ReviewSummaryContent cluster={cluster} /> </ExpandableSection> ); };Additionally, as noted in the
TableSummaryExpandable.tsxreview, consider wrappingtoggleContentin aContentcomponent for consistency:- toggleContent={'Cluster summary'} + toggleContent={<Content className="pf-v6-u-font-weight-bold">Cluster summary</Content>}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewCluster.css(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewPreflightChecks.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewSummary.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusterConfiguration/review/TableSummaryExpandable.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests
- GitHub Check: unit-tests
- GitHub Check: format
- GitHub Check: lint
- GitHub Check: circular-deps
- GitHub Check: translation-files
🔇 Additional comments (2)
libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewPreflightChecks.tsx (1)
189-191: LGTM! Minor UI alignment improvement.The addition of
pf-v6-u-text-align-startensures consistent left alignment of the "Preflight checks" label. The multi-line formatting improves readability without affecting functionality.libs/ui-lib/lib/ocm/components/clusterConfiguration/review/ReviewCluster.css (1)
21-21: Manually verify ExpandableSection toggle class names
Ensure the toggle button in PatternFly 6’s ExpandableSection renders.pf-v6-c-button__textand.pf-v6-c-button__icon; inspect the rendered DOM or PF6 source and adjust the CSS selectors if they differ.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, jgyselov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jgyselov
force-pushed
the
pf6_review_summary_styling
branch
from
5e835d6 to
453c5a5
Compare
|
New changes are detected. LGTM label has been removed. |
|
@jgyselov: This pull request references MGMT-21823 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 task to target the "4.21.0" version, but no target version was set. In 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. |
3 participants
https://issues.redhat.com/browse/MGMT-21823
PF5:
Before
After:
Summary by CodeRabbit
Style
Refactor