Migrate-Remaining-Toasts-to-Global-Banner-System Replace the copy act…#3665
Merged
Baalmart merged 4 commits intoJun 19, 2026
Conversation
…ions with the useClipboard hook
Contributor
|
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)
📝 WalkthroughWalkthroughThis PR migrates remaining ChangesToast-to-Banner Migration and Hierarchy
Secondary Sidebar Layout Adjustment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (3 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 |
Contributor
|
New azure vertex changes available for preview here |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/vertex/core/hooks/useRecentlyVisited.ts (1)
52-71:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
showBannerto the dependency array.The effect uses
showBanner(line 67) but doesn't include it in the dependency array (line 71). WhileshowBanneris memoized and stable, React best practices and ESLint exhaustive-deps require all values from component scope to be listed.✅ Suggested fix
- }, [storageKey]); + }, [storageKey, showBanner]);🤖 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/vertex/core/hooks/useRecentlyVisited.ts` around lines 52 - 71, The useEffect hook in useRecentlyVisited.ts uses the showBanner function on line 67 but fails to include it in the dependency array on line 71. Add showBanner to the dependency array alongside storageKey to ensure React's exhaustive-deps rule is satisfied and to maintain proper hook dependencies, even though showBanner is memoized and stable.src/vertex/components/features/home/network-visibility-card.tsx (1)
133-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the dialog open on failure for scoped banner visibility.
showBanner({ scoped: true })depends on the dialog’s banner slot, but the unconditional close/reset infinallyremoves that slot immediately, so the error is effectively hidden. Move dialog close/reset to the success path only.Suggested fix
try { await cohortsApi.updateCohortDetailsApi(pendingCohort._id, { visibility: targetVisibility, }); @@ if (isPersonalScope) { queryClient.invalidateQueries({ queryKey: ['personalUserCohorts', userId], }); } + setIsDialogOpen(false); + setPendingCohort(null); } catch (error) { console.error(error); showBanner({ severity: 'error', message: 'Failed to update network visibility', scoped: true }); } finally { setIsUpdating(false); - setIsDialogOpen(false); - setPendingCohort(null); }🤖 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/vertex/components/features/home/network-visibility-card.tsx` around lines 133 - 137, The finally block unconditionally closes the dialog with setIsDialogOpen(false) and clears pending state with setPendingCohort(null), which happens immediately even when an error occurs. Since showBanner with scoped: true requires the dialog to remain open to display the error banner in the dialog's banner slot, move the setIsDialogOpen(false) and setPendingCohort(null) calls from the finally block into the success path only (after the successful network visibility update completes), so the dialog stays open when displaying the error banner.
🤖 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.
Outside diff comments:
In `@src/vertex/components/features/home/network-visibility-card.tsx`:
- Around line 133-137: The finally block unconditionally closes the dialog with
setIsDialogOpen(false) and clears pending state with setPendingCohort(null),
which happens immediately even when an error occurs. Since showBanner with
scoped: true requires the dialog to remain open to display the error banner in
the dialog's banner slot, move the setIsDialogOpen(false) and
setPendingCohort(null) calls from the finally block into the success path only
(after the successful network visibility update completes), so the dialog stays
open when displaying the error banner.
In `@src/vertex/core/hooks/useRecentlyVisited.ts`:
- Around line 52-71: The useEffect hook in useRecentlyVisited.ts uses the
showBanner function on line 67 but fails to include it in the dependency array
on line 71. Add showBanner to the dependency array alongside storageKey to
ensure React's exhaustive-deps rule is satisfied and to maintain proper hook
dependencies, even though showBanner is memoized and stable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 032ef110-29cb-498c-94d0-e9016bbec048
📒 Files selected for processing (10)
src/vertex/app/changelog.mdsrc/vertex/app/providers.tsxsrc/vertex/components/features/claim/FileUploadParser.tsxsrc/vertex/components/features/cohorts/cohort-detail-card.tsxsrc/vertex/components/features/cohorts/cohort-organizations-card.tsxsrc/vertex/components/features/home/network-visibility-card.tsxsrc/vertex/components/features/networks/client-paginated-networks-table.tsxsrc/vertex/core/auth/authProvider.tsxsrc/vertex/core/hooks/useClipboard.tssrc/vertex/core/hooks/useRecentlyVisited.ts
Contributor
|
New azure vertex changes available for preview here |
Contributor
There was a problem hiding this comment.
Pull request overview
Completes the migration of remaining non-clipboard notifications from floating toasts to the useBanner / InfoBanner system, and consolidates direct clipboard writes under the shared useClipboard hook to standardize copy feedback behavior.
Changes:
- Migrates auth + core-hook notifications to global banners (
scoped: false) and feature-level failures to scoped banners (scoped: true). - Refactors multiple clipboard copy implementations to use
useClipboard, keeping success feedback as floating toasts. - Updates provider composition so auth-level components can safely call
useBanner, and documents the release in the changelog.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vertex/app/providers.tsx | Reorders providers so BannerProvider wraps AuthProvider, enabling banner usage in auth-level components. |
| src/vertex/core/auth/authProvider.tsx | Replaces ReusableToast auth events with global banners for session/account/network-related messages. |
| src/vertex/core/hooks/useRecentlyVisited.ts | Converts localStorage read failure notification to a global banner. |
| src/vertex/core/hooks/useClipboard.ts | Uses toast.success for copy success while routing copy failures to banners. |
| src/vertex/components/features/home/network-visibility-card.tsx | Converts network visibility mutation failure from toast to scoped banner. |
| src/vertex/components/features/claim/FileUploadParser.tsx | Replaces multiple validation/error toasts with scoped banners and adds a BannerSlot in the column mapper UI. |
| src/vertex/components/features/cohorts/cohort-detail-card.tsx | Replaces inline clipboard handling with useClipboard. |
| src/vertex/components/features/cohorts/cohort-organizations-card.tsx | Removes manual clipboard/toast logic and uses useClipboard. |
| src/vertex/components/features/networks/client-paginated-networks-table.tsx | Refactors copy-to-clipboard banner logic to use useClipboard. |
| src/vertex/app/changelog.md | Adds Version 2.0.7 release notes covering the migration and clipboard consolidation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| showBanner({ severity: 'success', message: 'Connection restored.', scoped: false }); | ||
| } | ||
| }, [isOnline, cachedUser, fetchStatus]); |
| showBanner({ severity: 'error', message: `Could not load user details: ${getApiErrorMessage(error)}`, scoped: false }); | ||
| } | ||
| } | ||
| }, [isError, error, isOnline, cachedUser, fetchStatus]); |
Comment on lines
+60
to
64
| File: {filePreview.fileName} | ||
| </p> | ||
| </div> | ||
| <BannerSlot /> | ||
| </div> |
| message: 'Your session has expired. Please log in again.', | ||
| type: 'ERROR', | ||
| }); | ||
| showBanner({ severity: 'error', message: 'Your session has expired. Please log in again.', scoped: false }); |
Contributor
|
New azure vertex changes available for preview here |
Contributor
|
New azure vertex changes available for preview here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of Changes (What does this PR do?)
Completes the migration of all remaining non-copy action notifications from floating
ReusableToastcalls to the context-awareInfoBannersystem (useBanner). Also consolidates all directnavigator.clipboard.writeTextusages under the shareduseClipboardhook.GlobalBannerContainerfor app-wide events, or scoped inside dialogs/cards via<BannerSlot />.Changes by file:
app/providers.tsx— MovedBannerProvideraboveAuthProviderso that core auth components (UserDataFetcher,AuthWrapper) can calluseBanner. Previously they were outside the provider boundary.core/auth/authProvider.tsx— Replaced 4ReusableToastcalls withshowBanner({ scoped: false }): "Connection restored", "Could not load user details", "Your account has been deleted", "Your session has expired".core/hooks/useRecentlyVisited.ts— ReplacedlocalStorageread failure toast with a global banner.core/hooks/useClipboard.ts— Success copies now usetoast.success(floating, intentional for clipboard feedback); copy errors now useshowBanner.components/features/home/network-visibility-card.tsx— "Failed to update network visibility" now renders as a scoped banner inside the confirmation dialog's existing<BannerSlot />.components/features/claim/FileUploadParser.tsx— Replaced 6 error toasts with scoped banners. Added<BannerSlot />insideBulkClaimColumnMapperheader and threadedshowBanneras a prop.components/features/cohorts/cohort-detail-card.tsx— Replaced inline clipboard try/catch withuseClipboardhook.components/features/cohorts/cohort-organizations-card.tsx— RemovedReusableToastimport and manualhandleCopyId; replaced withuseClipboardhook.components/features/networks/client-paginated-networks-table.tsx— Removed inlineuseBannerclipboard pattern; replaced withuseClipboardhook.Preserved (intentionally not migrated):
ReusableInputField.tsxandcohort-organizations-card.tsxcopy actions — per spec, clipboard feedback should remain as floating toasts.Status of maturity (all need to be checked before merging):
How should this be manually tested?
Global banners (auth events):
Scoped banners — Network Visibility Card:
3. Navigate to Home and find the Network Visibility card.
4. Toggle any cohort's visibility switch and click "Confirm" in the dialog.
5. To force an error: disable network in DevTools before confirming — confirm "Failed to update network visibility" appears inside the dialog, not as a floating toast.
Scoped banners — File Upload Parser:
6. Navigate to the Claim Devices page.
7. Attempt to import a
.txtfile — confirm "Invalid file format" error appears inside the upload modal.8. Attempt to import a file over 5MB — confirm "File too large" appears inside the modal.
9. Import a valid CSV, map both columns to the same field, click Import — confirm "Device name and claim token must be in different columns" appears inside the column mapper modal.
Clipboard hook consolidation:
10. On any cohort detail page, click the copy icon next to the Cohort ID — confirm a floating toast appears (not a banner).
11. On the Cohort Organizations card, click the copy icon next to an Org ID — confirm floating toast.
12. On the Admin → Networks page, click the copy icon on any network row — confirm floating toast.
What are the relevant tickets?
Summary by CodeRabbit