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

Show plugins in onboarding #4159

Closed
wants to merge 23 commits into from
Closed

Show plugins in onboarding #4159

wants to merge 23 commits into from

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Apr 29, 2021

Changes

This is a frontend-only change to onboarding section 2 UI, showing plugins. Builds on #4106, which made some plugins preinstalled and preenabled (currently only GeoIP, thinking about 1-3 more in the future).

Before

before

After

after

Checklist

  • Jest frontend tests
  • Cypress end-to-end tests

@Twixes Twixes changed the title Refactor plugin installation and reloading logic Show plugins in onboarding Apr 29, 2021
@Twixes Twixes changed the base branch from master to preinstall-plugins April 29, 2021 10:54
@timgl timgl temporarily deployed to posthog-pr-4159 April 29, 2021 10:56 Inactive
@Twixes Twixes temporarily deployed to posthog-pr-4159 April 29, 2021 13:23 Inactive
@Twixes Twixes temporarily deployed to posthog-pr-4159 April 29, 2021 15:06 Inactive
@Twixes Twixes temporarily deployed to posthog-pr-4159 April 29, 2021 15:45 Inactive
@Twixes Twixes temporarily deployed to posthog-pr-4159 April 30, 2021 00:24 Inactive
@Twixes Twixes removed the request for review from mariusandra April 30, 2021 00:27
@Twixes
Copy link
Member Author

Twixes commented Apr 30, 2021

Wellll, after reading https://github.com/PostHog/experiments/issues/2 I realized onboarding has been completely shut down as an experiment some time ago (even though it's still in code) – therefore #3764 seems to be outdated, and this PR in its current form is irrelevant. Should plugins end up in the new Project Home then - for instance? @kpthatsme @paolodamico

@paolodamico
Copy link
Contributor

So the onboarding experiment is shut down for now, but I do plan to re-scope it and bring back the setup page, so this is not wasted. We'll see how the project home evolves but I think that'll be more useful post-integration/post-setup. I can review this PR tomorrow or on Monday.

Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

I tried running this, but got this error:

image

Switching the listeners in personalizationLogic to lazily include organizationLogic (() => ({}) instead of {}) fixed that.

Then, if I click this button

image

... twice! Then it consistently kills the tab

Screenshot 2021-05-06 at 12 01 12

UX wise, I think we should just have a few checkboxes and then the "finish setup" button. I'm not sure the button to open plugins (and in a new tab) adds much. Let's instead have just one plugin there for now and add more later.

Other than that, looks good. If that crash gets fixed, I'd be Ok to merge.

Base automatically changed from preinstall-plugins to master May 7, 2021 01:02
@mariusandra
Copy link
Collaborator

Let's close this for now then... and reopen/recycle/regurgitate when there's a new place to put these toggles.

@mariusandra mariusandra deleted the onboard-plugins branch May 10, 2021 08:46
@Twixes Twixes restored the onboard-plugins branch May 10, 2021 09:26
@mariusandra mariusandra deleted the onboard-plugins branch June 22, 2021 11:35
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.

4 participants