Conversation
WalkthroughThe pull request modifies the Changes
Sequence DiagramsequenceDiagram
participant View as OnboardingView
participant State as animateContent
View->>State: Initialize as false
View->>View: onAppear triggered
View->>State: Set animateContent to true
State-->>View: Trigger opacity/offset animations
View->>View: Render animated elements
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (3)
LDKNodeMonday/View/Home/OnboardingView.swift (3)
32-45: Consider using relative offset values for better responsiveness.The fixed offset of 100 points might be too large on smaller devices. Consider using a relative value or GeometryReader.
- .offset(x: animateContent ? 0 : 100) + .offset(x: animateContent ? 0 : UIScreen.main.bounds.width * 0.25)
78-96: Consider staggering the button animations.The buttons currently animate simultaneously with the network settings (0.6s delay). A slightly longer delay for the buttons would create a more polished sequence.
- .animation(.easeOut(duration: 0.5).delay(0.6), value: animateContent) + .animation(.easeOut(duration: 0.5).delay(0.8), value: animateContent)
113-117: Consider specifying animation parameters in onAppear.While the current implementation works, explicit animation parameters in the withAnimation block would ensure consistent timing.
- withAnimation { + withAnimation(.easeOut(duration: 0.3)) { animateContent = true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
LDKNodeMonday/View/Home/OnboardingView.swift(4 hunks)
🔇 Additional comments (2)
LDKNodeMonday/View/Home/OnboardingView.swift (2)
19-19: LGTM! Well-structured state variable.The
animateContentstate variable is appropriately scoped and named.
55-71: Excellent animation choreography!Great use of:
- Spring animation for the logo with appropriate dampingFraction
- Cascading animations with delayed text appearance
- Group for efficient animation application
|
@danielnordh seemed uncontroversial (let me know if I'm wrong though!) so just merged. I'll want to fine-tune the timing a bit more and such, but I thought it was good enough to just do now. |
|
I have some general thoughts on animation, but let's discuss on our regular call. |
* Add new LoadingView * Use AppState for start/onboarding * Remove unused StartView and viewModel * Add ErrorView for showing startup errors * Move stored network to BackupInfo * Move serverURL into BackupInfo * Fix Picker issue where Server would not be set correctly * Refactor naming, user server instead of esplora for futureproofing * Remove unused parameter * WIP, separate viewModel for NetworkSettings * Swift format * Refactor networksettingsviewmodel * Use NetworkSettingsView from Settings (but doesn't yet change) * WIP restart, but UI doesn't respond * Swift format * Update LDKNodeMonday/Utilities/Constants.swift Refactor Co-authored-by: Matthew Ramsden <6657488+reez@users.noreply.github.com> * Fail when there is no available server * Fix error from github commit suggestion * WIP, guard for no saved network/server * Create global function for availableServers * Use general function for getting availableServers * Swift format * Handle errors * Implement WalletClient, network switching works * Move delete() logic to walletClient * Swift format * Don't stop node if not running * Set network and server on node start * Add getServer() * Use NetworkSettingsViewModel in onboarding, create wallet on WalletClient * Use NetworkSettingsViewModel for selected network and server * Swift format * Remove TODO * Add reset() to LightningClient to handle delete properly * Don't save on networkSettingsViewModel switch * Un-refactor SettingsView * Further un-refactoring of settingsview * Swift format * Coderabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Coderabbit suggestion * Simplify by not using NetworkSettingsViewModel * Simplify alert logic * Correct ldk-node version * Use appError from WalletClient * LDK-node package * Swift format * Remove unnecessary setting of tempNetwork when setting tempServer, handled in alert * Bugfix: start with saved server * Refactor, coderabbit suggestion * Swift format * Apply Onboarding animations from #157 * Remove unused parameter * Refactor handleRestart, partial Coderabbit suggestion --------- Co-authored-by: Matthew Ramsden <6657488+reez@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Description
Animate onboarding ui elements
Notes to the reviewers
Changelog notice
Checklists
All Submissions:
.swift-formatfileNew Features:
Bugfixes:
Summary by CodeRabbit
New Features
Style