ABI updates:#2985
Conversation
WalkthroughThis update refactors cluster page and header components to streamline header rendering and feature flag handling, modifies the credentials download flow to clear alerts before downloads, and updates component APIs to use children instead of custom props. Some components are removed or consolidated for clarity and maintainability. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CredentialsDownload
participant Alerts
participant Downloader
User->>CredentialsDownload: Click "Download Credentials"
CredentialsDownload->>Alerts: clearAlerts()
CredentialsDownload->>CredentialsDownload: setDownloading(true)
CredentialsDownload->>Downloader: download files
Downloader-->>CredentialsDownload: download complete
CredentialsDownload->>CredentialsDownload: setDownloading(false)
sequenceDiagram
participant ClusterPage
participant AssistedInstallerHeader
participant useFeature
ClusterPage->>AssistedInstallerHeader: Render header
AssistedInstallerHeader->>useFeature: Check feature flag
useFeature-->>AssistedInstallerHeader: Return flag status
AssistedInstallerHeader->>ClusterPage: Render header (with/without DeveloperPreview)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
libs/ui-lib/lib/ocm/components/clusterWizard/CredentialsDownload.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/ui-lib/lib/ocm/components/clusters/ClusterPage.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
approved
![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 (1)
libs/ui-lib/lib/ocm/components/clusters/NewClusterPage.tsx (1)
16-16: Improve type definition instead of using empty object type.The static analysis tool correctly identifies that
{}as a type should be avoided. Consider using a more explicit type definition.Apply this diff to improve the type definition:
-const NewClusterPageGeneric = ({ children }: React.PropsWithChildren<{}>) => { +const NewClusterPageGeneric = ({ children }: React.PropsWithChildren) => {Or if you need to be more explicit:
-const NewClusterPageGeneric = ({ children }: React.PropsWithChildren<{}>) => { +const NewClusterPageGeneric = ({ children }: { children?: ReactNode }) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 16-16: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/ui-lib/lib/ocm/components/clusterWizard/CredentialsDownload.tsx(2 hunks)libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusters/ClusterPage.tsx(1 hunks)libs/ui-lib/lib/ocm/components/clusters/NewClusterPage.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsx (1)
libs/ui-lib/lib/common/components/ui/DeveloperPreview.tsx (1)
DeveloperPreview(8-20)
libs/ui-lib/lib/ocm/components/clusters/NewClusterPage.tsx (1)
libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsx (1)
AssistedInstallerHeader(11-35)
🪛 Biome (1.9.4)
libs/ui-lib/lib/ocm/components/clusters/NewClusterPage.tsx
[error] 16-16: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (8)
libs/ui-lib/lib/ocm/components/clusters/ClusterPage.tsx (1)
208-210: LGTM! Consistent header implementation.The addition of
AssistedInstallerHeaderwrapped in aPageSectionwith light variant follows the same pattern established in other cluster pages and maintains UI consistency.libs/ui-lib/lib/ocm/components/clusterWizard/CredentialsDownload.tsx (2)
25-25: Good addition of clearAlerts functionality.Adding
clearAlertsto the destructureduseAlertshook enables proper alert management during the download process.
45-45: Excellent UX improvement - clearing alerts before downloads.Calling
clearAlerts()at the start of the download process ensures users see only relevant alerts from the current download attempt, preventing confusion from previous error messages.libs/ui-lib/lib/ocm/components/clusters/AssistedInstallerHeader.tsx (3)
3-9: Well-organized imports for feature flag functionality.The import additions are clean and properly organized, bringing in the necessary
DeveloperPreviewcomponent anduseFeaturehook for the new feature flag implementation.
12-12: Proper feature flag implementation.The feature flag check using
useFeature('ASSISTED_INSTALLER_SINGLE_CLUSTER_FEATURE')follows the established pattern and enables controlled rollout of the developer preview badge.
18-18: Clean conditional rendering of developer preview badge.The conditional rendering of
<DeveloperPreview />is implemented correctly and will only show the technical preview badge when the feature flag is enabled, aligning with the PR objectives.libs/ui-lib/lib/ocm/components/clusters/NewClusterPage.tsx (2)
27-30: Excellent consolidation of header rendering.Moving the
AssistedInstallerHeaderrendering insideNewClusterPageGenericwith a consistentPageSectionwrapper improves maintainability and ensures uniform header presentation across cluster pages.
47-49: Clean API improvement using children prop.The refactor to use the standard React
childrenprop instead of a custompageTitleSectionprop makes the component API more intuitive and follows React conventions.
- clear download alerts - add AI header + tech preview badge
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElayAharoni, rawagner 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 |
openshift-merge-bot
bot
merged commit 12ec61f
into
openshift-assisted:master
|
/cherry-pick release-4.19 |
|
@rawagner: new pull request created: #3003 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 kubernetes-sigs/prow repository. |
- clear download alerts - add AI header + tech preview badge
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor