Implement context aware global banner#3481
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a BannerContext with BannerProvider, a useBanner hook, BannerSlot for scoped banners, and GlobalBannerContainer for page-level banners, then integrates BannerProvider and GlobalBannerContainer into the root Providers tree. ChangesBanner Context and Provider Integration
Sequence Diagram(s)sequenceDiagram
participant Component
participant useBanner as useBanner Hook
participant BannerProvider
participant BannerSlot
participant GlobalBannerContainer
participant InfoBanner as Banner UI
Component->>useBanner: call showBanner(props)
useBanner->>BannerProvider: forward showBanner
BannerProvider->>BannerProvider: update state (isVisible, isScoped, props)
alt isScoped = true
BannerSlot->>InfoBanner: render scoped banner
else isScoped = false
GlobalBannerContainer->>InfoBanner: render global banner
end
Component->>useBanner: call hideBanner()
useBanner->>BannerProvider: forward hideBanner
BannerProvider->>BannerProvider: reset state (isVisible = false)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vertex/context/banner-context.tsx`:
- Around line 18-20: ShowBannerOptions currently advertises almost all
BannerProps but showBanner only persists and forwards severity, message and
title (plus scoped); narrow the type to match what is actually used or else
persist all props. Update the ShowBannerOptions declaration to exactly
Pick<BannerProps, 'severity' | 'message' | 'title'> & { scoped?: boolean } (or
include any additional fields you choose to support) and ensure the showBanner
implementation and the Banner render path (the function showBanner and the
Banner props passed where the banner is created) only expect and forward those
declared fields; alternatively if you want full parity, change showBanner to
persist and forward the complete set of BannerProps (except
dismissible/onDismiss) so the declared ShowBannerOptions matches runtime
behavior.
🪄 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: Pro
Run ID: ba59cd66-0eb8-4bf7-b3d1-2295050bd80e
📒 Files selected for processing (2)
src/vertex/app/providers.tsxsrc/vertex/context/banner-context.tsx
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codebmk
left a comment
There was a problem hiding this comment.
Brilliant work! Thank you for cooking. Cannot wait to see how you replace for all the other features @BwanikaRobert
Summary of Changes (What does this PR do?)
BannerContext,useBannerhook,BannerSlotandGlobalBannerContainer. The login page is the first test wrong password now shows an error banner inside the card, this was just for demonstration login page.tsx still uses the old Toast. No new dependencies, theBannercomponent was already there.Status of maturity (all need to be checked before merging):
How should this be manually tested?
-Navigate to
src/vertex/context/banner-context.tsx.-This exports banner context provider.
-Navigate to
src/vertex/app/providers.tsx.-The provider is wrapped around the component tree.
What are the relevant tickets?
Screenshots
Expected output after banner implementation
Summary by CodeRabbit