[INJIWEB-1802]Fixed the bug where the Coming soon toaster message was auto-dismissing.#478
[INJIWEB-1802]Fixed the bug where the Coming soon toaster message was auto-dismissing.#478iamlokanath wants to merge 1 commit intoinji:developfrom
Conversation
… auto-dismissing. Signed-off-by: Lkanath Panda <lokanthpanda128@gmai.com>
WalkthroughToast notifications in HomePage are configured to disable automatic closure with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
![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)
inji-web/src/pages/HomePage.tsx (1)
1-9: Minor formatting changes unrelated to the main fix.These import and spacing normalizations are stylistic improvements but unrelated to the toast auto-dismiss bug. Consider keeping PRs focused on a single objective for easier review and cleaner git history.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inji-web/src/__tests__/pages/HomePage.test.tsxinji-web/src/pages/HomePage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
inji-web/src/__tests__/pages/HomePage.test.tsx (1)
inji-web/src/utils/constants.ts (1)
LANDING_VISITED(65-65)
🔇 Additional comments (6)
inji-web/src/pages/HomePage.tsx (2)
65-67: JSX spacing normalization looks good.The added spaces in self-closing tags improve consistency and readability.
Also applies to: 70-70, 74-74
57-58: Implementation correctly addresses manual dismissal requirements.The
autoClose: falseconfiguration with react-toastify 10.0.5 properly disables automatic dismissal. Since no explicitcloseButton: falseoption is set, the default close button is shown automatically, allowing users to easily dismiss the toast. TheonClosecallback properly manages the toast visibility state. No further changes needed.inji-web/src/__tests__/pages/HomePage.test.tsx (4)
76-101: Test formatting improvements look good.The reformatted sessionStorage tests maintain the same functionality with cleaner structure and proper spy usage.
110-135: Test properly verifies single toast creation with autoClose disabled.The test correctly validates that:
- Multiple rapid clicks only trigger one toast
- The toast is configured with
autoClose: false- The
toastIdis properly set
149-178: Excellent test coverage for non-auto-closing behavior.This test effectively validates the core fix by:
- Triggering the toast
- Waiting 4 seconds (longer than typical auto-close duration)
- Verifying the toast remains present
- Confirming
autoClose: falsewas passed to toast.warningThe 4-second wait on Line 172 provides strong evidence that auto-dismissal is truly disabled.
180-234: Test correctly validates toast can be re-shown after manual dismissal.The test properly verifies that:
- After a toast is manually closed (via
onClosecallback)- A subsequent click can trigger a new toast
- The new toast also has
autoClose: falseThe use of fake timers and manual triggering of
onCloseaccurately simulates the real-world flow.
INJIWEB-1802.mp4 |
| import React, { useEffect, useState } from "react"; | ||
| import { HomeBanner } from "../components/Home/HomeBanner"; | ||
| import { HomeFeatures } from "../components/Home/HomeFeatures"; | ||
| import { HomeQuickTip } from "../components/Home/HomeQuickTip"; | ||
| import { toast } from "react-toastify"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { useLocation } from 'react-router-dom'; | ||
| import { LoginFailedModal } from '../components/Login/LoginFailedModal'; | ||
| import { LANDING_VISITED } from "../utils/constants"; |
There was a problem hiding this comment.
same applies here. let’s avoid whitespace or formatting-only changes
| const setItemSpy = jest.spyOn(window.sessionStorage.__proto__, 'setItem'); | ||
| renderComponent(); | ||
| expect(setItemSpy).toHaveBeenCalledWith(LANDING_VISITED, 'true'); | ||
| }); | ||
|
|
||
| test('logs a warning if sessionStorage.setItem fails', () => { | ||
| const setItemSpy = jest | ||
| .spyOn(window.sessionStorage.__proto__, 'setItem') | ||
| .mockImplementation(() => { | ||
| throw new Error('storage failed'); | ||
| }); | ||
| const setItemSpy = jest | ||
| .spyOn(window.sessionStorage.__proto__, 'setItem') | ||
| .mockImplementation(() => { | ||
| throw new Error('storage failed'); | ||
| }); | ||
|
|
||
| const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); | ||
| const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => { }); | ||
|
|
||
| renderComponent(); | ||
| renderComponent(); | ||
|
|
||
| expect(setItemSpy).toHaveBeenCalledWith(LANDING_VISITED, 'true'); | ||
| expect(warnSpy).toHaveBeenCalledWith( | ||
| 'Unable to access sessionStorage', | ||
| expect.any(Error) | ||
| ); | ||
| expect(setItemSpy).toHaveBeenCalledWith(LANDING_VISITED, 'true'); | ||
| expect(warnSpy).toHaveBeenCalledWith( | ||
| 'Unable to access sessionStorage', | ||
| expect.any(Error) | ||
| ); |
There was a problem hiding this comment.
Could these formatting updates be avoided and the changes be kept focused on the behavior? It will help keep the diff clean and easier to review.
Rudhhi-Shah-14
left a comment
Rudhhi-Shah-14
left a comment
There was a problem hiding this comment.
Please try to limit the changes in this PR to functional updates only.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.