Refactor skip button logic in StageFooter#344
Conversation
This should make the skip button work. Previously the skip button would go back and not forward.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughStageFooter is now ChangesStageFooter updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
I previously accidentally made a pull to main. This is the same code and pull request but just in dev. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Onboarding/Components/StageFooter.swift (2)
68-75: ⚡ Quick winConsider removing the redundant outer condition check.
The condition at line 68 checking
currentStage > 0is redundant because line 66 already ensures the button only appears whencurrentStage != 0. This creates unnecessary nesting without functional benefit.♻️ Proposed simplification
if !(currentStage == 0) { Button { - if currentStage > 0 { - if currentStage == 1 { - // Skip at stage 1 should advance, not go back - onContinue() - } else { - onBack() - } + if currentStage == 1 { + // Skip at stage 1 should advance, not go back + onContinue() + } else { + onBack() } } label: {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Onboarding/Components/StageFooter.swift` around lines 68 - 75, Remove the redundant outer `if currentStage > 0` check and simplify the control flow inside the StageFooter so the Skip handling directly checks `currentStage == 1` to call `onContinue()` and otherwise calls `onBack()`; update the Skip button handler in StageFooter.swift to call `onContinue()` when `currentStage == 1` and `onBack()` for all other non-zero stages (using the existing `currentStage`, `onContinue()`, and `onBack()` symbols).
10-10: Onboarding/Components/StageFooter.swift: remove redundantcurrentStage > 0guard in secondary button
- The secondary button is only shown when
currentStage != 0andOnboardingViewconstrainscurrentStageto0...7(advanceguards< 7,goBackguards> 0), so the innerif currentStage > 0 { ... }(line 68) is redundant. You can simplify the action toif currentStage == 1 { onContinue() } else { onBack() }.StageFooterhas no explicit@MainActor; the repo also doesn’t annotatestruct ...: View/ViewModifiertypes with@MainActor, so this matches the project’s convention for SwiftUI view isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Onboarding/Components/StageFooter.swift` at line 10, The secondary button in StageFooter contains a redundant `if currentStage > 0` check; since the button is only rendered when `currentStage != 0` and the parent constrains stages, replace the inner guarded branch in the secondary button's action with a simple conditional: `if currentStage == 1 { onContinue() } else { onBack() }`, removing the extra `if currentStage > 0 { ... }` wrapper; keep StageFooter as-is without adding `@MainActor` annotations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Onboarding/Components/StageFooter.swift`:
- Around line 68-75: Remove the redundant outer `if currentStage > 0` check and
simplify the control flow inside the StageFooter so the Skip handling directly
checks `currentStage == 1` to call `onContinue()` and otherwise calls
`onBack()`; update the Skip button handler in StageFooter.swift to call
`onContinue()` when `currentStage == 1` and `onBack()` for all other non-zero
stages (using the existing `currentStage`, `onContinue()`, and `onBack()`
symbols).
- Line 10: The secondary button in StageFooter contains a redundant `if
currentStage > 0` check; since the button is only rendered when `currentStage !=
0` and the parent constrains stages, replace the inner guarded branch in the
secondary button's action with a simple conditional: `if currentStage == 1 {
onContinue() } else { onBack() }`, removing the extra `if currentStage > 0 { ...
}` wrapper; keep StageFooter as-is without adding `@MainActor` annotations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1d5a28f-1dfb-470c-b891-ccfcb32d49b4
📒 Files selected for processing (1)
Onboarding/Components/StageFooter.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Onboarding/Components/StageFooter.swift (1)
10-10:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
@MainActorannotation for thread safety.The struct is missing the
@MainActorannotation required by the coding guidelines. As per coding guidelines, all Swift code must be confined to@MainActorfor thread safety across the entire codebase.🔒 Proposed fix to add `@MainActor`
+@MainActor struct StageFooter: View {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Onboarding/Components/StageFooter.swift` at line 10, Add the `@MainActor` annotation to the StageFooter view struct to ensure UI/thread-safety; locate the struct declaration "struct StageFooter: View" and prepend the `@MainActor` attribute (i.e., change the declaration to annotate StageFooter with `@MainActor`) so the view and its methods run on the main actor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Onboarding/Components/StageFooter.swift`:
- Line 10: Add the `@MainActor` annotation to the StageFooter view struct to
ensure UI/thread-safety; locate the struct declaration "struct StageFooter:
View" and prepend the `@MainActor` attribute (i.e., change the declaration to
annotate StageFooter with `@MainActor`) so the view and its methods run on the
main actor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ff79e36-8711-4d46-9115-3f7db00f835b
📒 Files selected for processing (1)
Onboarding/Components/StageFooter.swift
|
Note: last two commit messages were generated by AI. Second commit used AI by inputting the coderabbit AI prompt just to help me understand what coderabbit was saying a bit more. Actual changes were made by me. |
|
@jonathancaudill I could make a discord server for Nook so that there is a more informal chat area and a way to easily contact contributers and other supporters. What is your discord btw? |
|
@wemvora there already is one!! should be in the gh description or on the website I think. TBH, I'm a full time tech employee now so contributing anything outside of small open source projects is nigh impossible these days. Will probably be looking for someone to take over maintenance. |
|
@jonathancaudill yes I could help with maintenance and keep the dc up to date with events and stuff |
2 participants
This should make the skip button work. Previously the skip button would go back and not forward.
Summary by CodeRabbit