Skip to content

SSF-124 Replacing Browser Alerts#103

Open
Juwang110 wants to merge 7 commits intomainfrom
jw/SSF-124-replacing-native-alerts
Open

SSF-124 Replacing Browser Alerts#103
Juwang110 wants to merge 7 commits intomainfrom
jw/SSF-124-replacing-native-alerts

Conversation

@Juwang110
Copy link

ℹ️ Issue

Closes https://vidushimisra.atlassian.net/jira/software/projects/SSF/boards/1?selectedIssue=SSF-124

📝 Description

This PR simply replaced native browser alerts with chakra alerts for better visuals. It only changes alerts that are on pages with updated designs according to the figma so some native alert() usages still exist but those will be updated when those pages get visually updated.

To make this change, I added a new reusable floatingAlert component

The routes effected are:

/admin-donation, /admin-order-management, /request-form/:pantryId

✔️ Verification

I verified each alert showed up as applicable.

image

🏕️ (Optional) Future Work / Notes

N/A

Copy link
Member

@maxn990 maxn990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think you may have added slightly more padding/spacing than is on the figma, but otherwise looks really good. I just had a quick question too that I added in comments, but otherwise it looks really good!!

Comment on lines 70 to 72
{alertMessage && (
<FloatingAlert message={alertMessage} status="error" timeout={6000} />
)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason this alert is in a portal and none of the others are?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey max! To be honest no reason, I moved it out

@Juwang110 Juwang110 requested a review from maxn990 February 14, 2026 22:29
Copy link
Member

@maxn990 maxn990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! 🥳 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants