Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: onboarding anon user fetching actions #4150

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Conversation

sshanzel
Copy link
Member

@sshanzel sshanzel commented Feb 5, 2025

Changes

  • The loading is stuck on certain scenarios.
  • One condition of when to show the spinner is when the onboarding is ready.
  • One of the criteria is to check whether actions is fetched. The problem is, anon users don't have actions yet, hence the query is always not fetched/ready.

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://fix-actions-ready.preview.app.daily.dev

Copy link

vercel bot commented Feb 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
daily-webapp ✅ Ready (Inspect) Visit Preview Feb 5, 2025 2:53pm
1 Skipped Deployment
Name Status Preview Updated (UTC)
storybook ⬜️ Ignored (Inspect) Feb 5, 2025 2:53pm

@@ -33,7 +34,7 @@ export const useOnboarding = (): UseOnboarding => {

return {
shouldShowAuthBanner,
isOnboardingReady: isActionsFetched && isAuthReady,
isOnboardingReady: isActionsReady && isAuthReady,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bit unclear what we want to do, since isAuthReady is specified both here and in above condition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe the condition can be simplified, but yeah, the end result should be the same. Let me refactor this to be a 1-liner.

@sshanzel sshanzel requested a review from capJavert February 5, 2025 14:51
Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this makes more sense to me, nice 👍

@sshanzel sshanzel merged commit f35bdc3 into main Feb 5, 2025
10 checks passed
@sshanzel sshanzel deleted the fix-actions-ready branch February 5, 2025 14:55
sshanzel added a commit that referenced this pull request Feb 5, 2025
sshanzel added a commit that referenced this pull request Feb 5, 2025
sshanzel added a commit that referenced this pull request Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants