Conversation
WalkthroughMoves PulseToast initialization and rendering from HomeScreen to AppWrapper. AppWrapper now shows the beta toast on mount and hides it on close. HomeScreen no longer references PulseToast. Tests added to verify toast visibility on open and dismissal behavior. No exported/public APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router as Router
participant App as AppWrapper
participant Toast as PulseToast
Router->>App: Mount AppWrapper
activate App
App->>App: useEffect on mount sets showBetaToast = true
App->>Toast: Render PulseToast (visible)
deactivate App
User->>Toast: Click Close
Toast-->>App: onClose callback
App->>App: set showBetaToast = false
App-->>Toast: Unmount PulseToast (after animation)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Deploying x with
|
| Latest commit: |
f96776d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://65ab2db8.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3749-pulse-beta-tag.x-e62.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/apps/pulse/components/App/tests/AppWrapper.test.tsx (1)
299-320: Prefer testing-library async utilities over manual setTimeout.The test works but could be more robust and idiomatic. Consider using
waitForElementToBeRemovedfrom@testing-library/reactanduserEventfor interactions instead of manual Promise/setTimeout and direct.click().Apply this diff to improve the test:
+import { render, screen, waitForElementToBeRemoved } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; it('allows closing the beta toast', async () => { + const user = userEvent.setup(); renderWithProviders( <MemoryRouter initialEntries={['/']}> <AppWrapper /> </MemoryRouter> ); expect(screen.getByText('Welcome to Pulse (beta)')).toBeInTheDocument(); // Close the toast const closeButton = screen.getByLabelText('Close'); - closeButton.click(); + await user.click(closeButton); - // Wait for the animation to complete (300ms delay + some buffer) - await new Promise((resolve) => { - setTimeout(resolve, 400); - }); - - expect( - screen.queryByText('Welcome to Pulse (beta)') - ).not.toBeInTheDocument(); + // Wait for toast to be removed after animation + await waitForElementToBeRemoved(() => + screen.queryByText('Welcome to Pulse (beta)') + ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/pulse/components/App/AppWrapper.tsx(4 hunks)src/apps/pulse/components/App/HomeScreen.tsx(0 hunks)src/apps/pulse/components/App/tests/AppWrapper.test.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/apps/pulse/components/App/HomeScreen.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pulse/components/App/tests/AppWrapper.test.tsx (1)
src/apps/pulse/components/App/AppWrapper.tsx (1)
AppWrapper(12-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (5)
src/apps/pulse/components/App/tests/AppWrapper.test.tsx (1)
289-297: LGTM!The test correctly verifies that the beta toast appears when the Pulse app opens. The implementation is straightforward and validates the core requirement.
src/apps/pulse/components/App/AppWrapper.tsx (4)
9-9: LGTM!The import is correctly added and follows the project's import structure.
18-18: LGTM!State initialization to
falseis appropriate, ensuring the toast only displays after the mount effect runs.
43-46: Verify intended "fresh render" behavior.The PR title mentions "only on fresh render," but the implementation shows the toast on every component mount (due to the empty dependency array). If "fresh render" means the first-ever app launch (not just component mount), you'd need persistence (e.g., localStorage or sessionStorage) to track whether the user has already seen the toast.
Current behavior: Toast appears every time AppWrapper mounts (e.g., navigating away and back).
Intended behavior based on comment: "every time Pulse app opens" suggests current implementation is correct.
Please confirm if this matches your requirements.
69-99: LGTM!The rendering logic correctly positions the PulseToast above both view paths. The onClose handler properly manages toast dismissal, and the fragment wrapper cleanly accommodates the additional UI element without disrupting existing logic.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Tests