Update shipping management module to use info banners#3627
Conversation
📝 WalkthroughWalkthroughShippingLabelPrintModal integrates the ChangesShipping Label Print Modal Banner Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR replaces blocking alert() calls in the shipping label print flow with the app’s banner/toast system to provide non-blocking, scoped error feedback when printing can’t proceed.
Changes:
- Add
useBannerto the shipping label print modal. - Replace pop-up blocked and invalid QR URL
alert()messages withshowBanner(...)errors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isValidDataUrl = (url: string) => { | ||
| return url.startsWith('data:image/') || /^https?:\/\//i.test(url); | ||
| }; |
|
New azure docs changes available for preview here |
|
New azure website changes available for preview here |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vertex/components/features/shipping/ShippingLabelPrintModal.tsx (1)
196-254:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Missing BannerSlot — scoped banners won't render.
The component calls
showBanner({ scoped: true })at lines 28 and 34, but never renders theBannerSlotcomponent inside the modal. According to the banner context contract, scoped banners only appear whereBannerSlotis placed. Without it, your error messages will be silently dropped.Add
BannerSlotat the top of the dialog content, immediately after theReusableDialogopening tag (before line 220).🔧 Proposed fix
+import { useBanner } from '`@/context/banner-context`'; + const ShippingLabelPrintModal: React.FC<ShippingLabelPrintModalProps> = ({ labels, isOpen, onClose }) => { - const { showBanner } = useBanner(); + const { showBanner, BannerSlot } = useBanner(); // ... handlePrint logic ... return ( <ReusableDialog isOpen={isOpen} onClose={onClose} title="Shipping Labels" // ... other props ... > + <BannerSlot /> <div className="print-container"> {labels.map((label, index) => (🤖 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/shipping/ShippingLabelPrintModal.tsx` around lines 196 - 254, The modal invokes showBanner({ scoped: true }) but never renders BannerSlot, so scoped banners are dropped; inside the ShippingLabelPrintModal add the BannerSlot component into the ReusableDialog content area immediately after the ReusableDialog opening tag (i.e., at the top of the returned JSX inside ReusableDialog) so scoped banners will render; ensure you import BannerSlot where ShippingLabelPrintModal uses ReusableDialog and place a single <BannerSlot /> before the existing .print-container content.
🤖 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/components/features/shipping/ShippingLabelPrintModal.tsx`:
- Around line 32-36: The pop-up opened via window.open is left open when QR
validation fails; capture the window reference (e.g., const printWindow =
window.open(...)) and, inside the invalid QR handling branch (the
invalidLabels.length > 0 block that uses isValidDataUrl and showBanner), call
printWindow?.close() (guarding for null) before showing the banner and returning
so the empty pop-up is closed on validation failure.
---
Outside diff comments:
In `@src/vertex/components/features/shipping/ShippingLabelPrintModal.tsx`:
- Around line 196-254: The modal invokes showBanner({ scoped: true }) but never
renders BannerSlot, so scoped banners are dropped; inside the
ShippingLabelPrintModal add the BannerSlot component into the ReusableDialog
content area immediately after the ReusableDialog opening tag (i.e., at the top
of the returned JSX inside ReusableDialog) so scoped banners will render; ensure
you import BannerSlot where ShippingLabelPrintModal uses ReusableDialog and
place a single <BannerSlot /> before the existing .print-container content.
🪄 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: 05099d0b-3b14-4f87-8ed7-4f1cf517ad11
📒 Files selected for processing (1)
src/vertex/components/features/shipping/ShippingLabelPrintModal.tsx
|
New azure analytics_platform changes available for preview here |
|
New azure vertex changes available for preview here |
|
New azure website changes available for preview here |
|
New azure docs changes available for preview here |
|
New azure vertex changes available for preview here |
|
New azure analytics_platform changes available for preview here |
|
@BwanikaRobert pr is still marked as draft. Are you still working on it? Any blockers? |
|
New azure vertex changes available for preview here |
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 (1)
src/vertex/components/features/shipping/ShippingLabelPrintModal.tsx (1)
29-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
BannerSlotin the modal content so scoped errors are actually visible.
showBanner(... scoped: true)is correct, but this component doesn’t render<BannerSlot />, so these errors can be dropped instead of appearing inline in the dialog.Suggested fix
- const { showBanner } = useBanner(); + const { showBanner, BannerSlot } = useBanner(); ... <ReusableDialog isOpen={isOpen} onClose={onClose} @@ > + <BannerSlot /> <div className="print-container"> {labels.map((label, index) => (Based on learnings from the provided PR objectives and banner-context contract, scoped banners must have a
BannerSlotin the active dialog to render inline.Also applies to: 222-223
🤖 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/shipping/ShippingLabelPrintModal.tsx` around lines 29 - 37, The ShippingLabelPrintModal component is using showBanner() with scoped: true to display error messages, but the component does not render a BannerSlot component. Without the BannerSlot in the modal content, scoped banner errors will not be displayed inline in the dialog. Add a BannerSlot component to the modal's JSX content (in the return statement) to ensure that scoped error banners from showBanner() calls are visible to users when validation failures occur.
🤖 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/shipping/ShippingLabelPrintModal.tsx`:
- Around line 29-37: The ShippingLabelPrintModal component is using showBanner()
with scoped: true to display error messages, but the component does not render a
BannerSlot component. Without the BannerSlot in the modal content, scoped banner
errors will not be displayed inline in the dialog. Add a BannerSlot component to
the modal's JSX content (in the return statement) to ensure that scoped error
banners from showBanner() calls are visible to users when validation failures
occur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df2685c9-e516-48a2-8e95-fc35243c3527
📒 Files selected for processing (3)
src/vertex/app/(authenticated)/admin/shipping/[batchId]/page.tsxsrc/vertex/app/changelog.mdsrc/vertex/components/features/shipping/ShippingLabelPrintModal.tsx
💤 Files with no reviewable changes (1)
- src/vertex/app/(authenticated)/admin/shipping/[batchId]/page.tsx
✅ Files skipped from review due to trivial changes (1)
- src/vertex/app/changelog.md
Summary of Changes (What does this PR do?)
Migrates all user-facing notifications in the Shipping Management modules away from native
alert()calls andReusableToastto the centraliseduseBanner/useBannerWithDelay.Shipping Management
ShippingLabelPrintModal.tsx— replaced twoalert()calls (pop-up blocker warning, invalid QR URL error) withshowBanner({ severity: 'error', scoped: true })so errors appear inline inside the print dialog rather than as native browser dialogs.PRsStatus of maturity (all need to be checked before merging):
How should this be manually tested?
alert()).What are the relevant tickets?
Screenshots
Summary by CodeRabbit