fix: remove navbar during onboarding animation#2836
Conversation
Console (appwrite/console)Project ID: Tip Schedule functions to run as often as every minute with cron expressions |
WalkthroughA new boolean Svelte store Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/`(console)/onboarding/create-project/+page.svelte:
- Around line 61-67: The setTimeout async callback that calls
invalidate(Dependencies.ACCOUNT) and goto(...) can throw and currently leaves
showOnboardingAnimation true; wrap the body of the setTimeout callback in a
try/catch/finally (or try { await invalidate(...); await goto(...); } catch
(err) { console.error(err) /* or use app logger */ } finally {
showOnboardingAnimation.set(false) }) so errors from invalidate or goto are
handled and the onboarding animation flag is always cleared; reference the
setTimeout callback, showOnboardingAnimation, invalidate, goto, and
Dependencies.ACCOUNT when applying the change.
🧹 Nitpick comments (1)
src/lib/layout/shell.svelte (1)
8-10: Minor: Extra blank line.Line 10 introduces an extra blank line after the import statement. This is a very minor formatting inconsistency.
| showOnboardingAnimation.set(true); | ||
|
|
||
| setTimeout(async () => { | ||
| await invalidate(Dependencies.ACCOUNT); | ||
| goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`); | ||
| await goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`); | ||
| showOnboardingAnimation.set(false); | ||
| }, 3000); |
There was a problem hiding this comment.
Missing error handling in navigation callback could leave UI in broken state.
If invalidate() or goto() throws inside the setTimeout callback, the error is unhandled and showOnboardingAnimation remains true, permanently hiding the navbar and sidebar. The outer catch block only handles project creation errors.
🛡️ Proposed fix to add error handling
startAnimation = true;
showOnboardingAnimation.set(true);
setTimeout(async () => {
- await invalidate(Dependencies.ACCOUNT);
- await goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`);
- showOnboardingAnimation.set(false);
+ try {
+ await invalidate(Dependencies.ACCOUNT);
+ await goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`);
+ } finally {
+ showOnboardingAnimation.set(false);
+ }
}, 3000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| showOnboardingAnimation.set(true); | |
| setTimeout(async () => { | |
| await invalidate(Dependencies.ACCOUNT); | |
| goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`); | |
| await goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`); | |
| showOnboardingAnimation.set(false); | |
| }, 3000); | |
| showOnboardingAnimation.set(true); | |
| setTimeout(async () => { | |
| try { | |
| await invalidate(Dependencies.ACCOUNT); | |
| await goto(`${base}/project-${project.region ?? 'default'}-${project.$id}`); | |
| } finally { | |
| showOnboardingAnimation.set(false); | |
| } | |
| }, 3000); |
🤖 Prompt for AI Agents
In `@src/routes/`(console)/onboarding/create-project/+page.svelte around lines 61
- 67, The setTimeout async callback that calls invalidate(Dependencies.ACCOUNT)
and goto(...) can throw and currently leaves showOnboardingAnimation true; wrap
the body of the setTimeout callback in a try/catch/finally (or try { await
invalidate(...); await goto(...); } catch (err) { console.error(err) /* or use
app logger */ } finally { showOnboardingAnimation.set(false) }) so errors from
invalidate or goto are handled and the onboarding animation flag is always
cleared; reference the setTimeout callback, showOnboardingAnimation, invalidate,
goto, and Dependencies.ACCOUNT when applying the change.
2 participants
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit