Call onboarding dialog only once #144

Open

jose wants to merge 1 commits from onboarding-dialog-fix into master

jose commented 1 day ago

Owner

Removed earlier call to onboarding dialog, choosing to call it only at the end to make way for any error notifications

jose added 1 commit 1 day ago

3e6e959b19 Call onboarding dialog only once

- Removed earlier call to onboarding dialog, choosing to call it only at the end to make way for any error notifications

trentlarson reviewed 1 day ago

src/views/HomeView.vue

												
					    }
				
					    if (!settings.finishedOnboarding) {
				
					    if (this.needsOnboarding) {

trentlarson commented 1 day ago

Poster

Owner

Note that there is a hole in the logic: needsOnboarding is never set to false after they have seen it. (It may not cause a problem because we always recheck, but it does mean that this new variable doesn't always reflect the true state of the user.)

Another possible complication is: after they finish onboarding, they no longer need onboarding, so this method will always check the DB again. (That doesn't seem like it'll cause a problem; I just mention it because I think it's not intended because we keep checking the flag.)

This is a common problem with new state variables. If you ever get a chance to play with functional programming (eg. Haskell, Scheme, LISP) then it'll be interesting how it makes you think differently... and you're able to avoid a lot of state variables in the classes because you always pass all the variables you need.

Anyway, I think it would be fine to eliminate the needsOnboarding flag and always check with retrieveSettingsForActiveAccount in here. That way we don't have to worry about setting a variable. (Another more efficient option is to eliminate that additional DB check here and pass in the flag to the checkOnboarding method from above -- but if it makes things more complicated and harder to understand then it's not worthwhile.)

This pull request can be merged automatically.

You are not authorized to merge this pull request.

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.