Replace toast with message title for Pulse beta tag#424
Conversation
WalkthroughRemoved Pulse beta toast logic from AppWrapper and related tests, simplified AppWrapper render to a direct Search/HomeScreen ternary, and added a static beta notice paragraph in HomeScreen’s non-preview state. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant AW as AppWrapper
participant S as Search
participant H as HomeScreen
%% Old vs New high-level flow (toast removed)
rect rgb(245,245,255)
note over AW: New flow
U->>AW: Open Pulse
AW-->>AW: isSearching?
alt Searching
AW->>S: Render Search
else Not searching
AW->>H: Render HomeScreen
end
end
rect rgb(255,245,245)
note over AW: Old flow (removed)
U->>AW: Open Pulse
AW-->>AW: init beta toast state/effect
AW-->>U: Show PulseToast (beta)
AW-->>AW: isSearching?
alt Searching
AW->>S: Render Search
else Not searching
AW->>H: Render HomeScreen
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: |
2e2e8b1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d46d0bf7.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/HomeScreen.tsx (1)
823-826: Beta notice added successfully, but consider UX and styling improvements.The beta notice paragraph is now displayed in the home screen. However, there are a few considerations:
Inconsistent Summary: The AI summary mentions the notice appears "in two places," but the code shows only one instance at lines 823-826. This suggests the summary may be inaccurate.
User Experience: The notice displays every time users return to the home state, which could become repetitive for returning users. Consider showing it only once per session using localStorage or sessionStorage.
Display Property: Using
flexon a<p>element is unusual since paragraph elements are typically block-level. Consider usingblockinstead or changing to a<div>if flex layout is needed for child elements.Optional refactor to show the notice only once per session:
+import { useEffect, useState } from 'react'; + +const BETA_NOTICE_KEY = 'pulse-beta-notice-seen'; + export default function HomeScreen(props: HomeScreenProps) { + const [showBetaNotice, setShowBetaNotice] = useState(() => { + return !sessionStorage.getItem(BETA_NOTICE_KEY); + }); + + useEffect(() => { + if (showBetaNotice) { + sessionStorage.setItem(BETA_NOTICE_KEY, 'true'); + } + }, [showBetaNotice]); ... return ( <> - <p className="flex text-base font-normal text-white/[.5] max-w-[442px] text-center mb-6"> - You're trying out the beta version of Pulse: expect improvements - ahead. Thank you. - </p> + {showBetaNotice && ( + <p className="block text-base font-normal text-white/[.5] max-w-[442px] text-center mb-6"> + You're trying out the beta version of Pulse: expect improvements + ahead. Thank you. + </p> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/apps/pulse/components/App/AppWrapper.tsx(1 hunks)src/apps/pulse/components/App/HomeScreen.tsx(1 hunks)src/apps/pulse/components/App/tests/AppWrapper.test.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/apps/pulse/components/App/tests/AppWrapper.test.tsx
👮 Files not reviewed due to content moderation or server errors (2)
- src/apps/pulse/components/App/HomeScreen.tsx
- src/apps/pulse/components/App/AppWrapper.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pulse/components/App/AppWrapper.tsx (2)
src/apps/pulse/components/Search/Search.tsx (1)
Search(64-451)src/apps/pulse/components/App/HomeScreen.tsx (1)
HomeScreen(80-991)
⏰ 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: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (5)
src/apps/pulse/components/App/HomeScreen.tsx (2)
823-826: LGTM! Clean implementation of static beta notice.The beta notice is well-positioned before the search button and provides clear messaging to users. The styling is appropriate with good visual hierarchy using semi-transparent text.
823-826: LGTM! Clean implementation of the beta notice.The beta message is appropriately placed and styled. Note that it only appears in the default state (not during preview transactions or transaction status screens), which seems intentional to avoid cluttering those flows.
src/apps/pulse/components/App/AppWrapper.tsx (3)
62-87: LGTM! Excellent simplification.Removing the toast wrapper and beta state management significantly simplifies the component logic. The direct ternary between Search and HomeScreen is clean and maintainable.
62-87: LGTM! Clean refactor that improves code clarity.The simplified render logic with a direct ternary is more readable and maintainable than the previous fragment-wrapped conditional rendering with the toast wrapper. All component props are correctly passed through.
62-87: LGTM! Clean simplification of render logic.The removal of the beta toast wrapper and simplification to a direct ternary operator improves code readability while maintaining all necessary functionality. All props are correctly passed to both the Search and HomeScreen components.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit