feat: in app prompts experiment #10956

kappa90 · 2022-07-25T15:48:18Z

Problem

Following this RFC, this PR is an experimental implementation of customizable in-app prompts

Changes

There is a new endpoint, PATCH /prompts/my_prompts, which is used to receive the prompt sequences that should be shown to a person, as well as update and receive the state for each sequence (i.e. which prompts have already been displayed for the current person)
A new logic inAppPromptLogic takes care of syncing the local person prompt state with the backend, as well as deciding which prompts to display
For now, prompts are displayed conditionally based on the current path, using wildcards (e.g. /dashboard/*). We will run this as an experiment so we can use the experiment cohorts logic to show this conditionally to certain persons, but in the future this could be a native feature of prompts (by extending feature flags to carry a prompt configuration payload).
Currently the only type of implemented prompt is a new type of tooltip called LemonActionableTooltip, which can be displayed next to UI elements, can have custom icons and actionable buttons (see screenshots below)
The current flow is based on this copy...
...however I feel like we need to make each sequence of tooltips independent of the navigation order a certain user might follow. We can't enforce users to discover the app in a specific way, so we should just create standalone product tours for each page, regardless of the direction they take
There is a new button within the Help Menu, where you can re-trigger a product tour for any page where there is one.
For the experiment, the plan is to run this only for users which sign-up from the start of the experimentation onwards

To-do:

Finish writing kea tests
@joethreepwood let's make the copy path independent, as mentioned above
@clarkus we reviewed this together but take a look anyways

Here's a video preview of the flow

How did you test this code?

New tests for the frontend logic

joesaunderson · 2022-07-25T19:32:27Z

Heavily heavily heavily invested in this feature (from Mention Me, an enterprise customer).

benjackwhite

Bunch of small/medium comments. Only thing really missing for approval would be some tests, especially around the api endpoint and the logic

benjackwhite · 2022-07-26T06:49:39Z

frontend/src/lib/components/HelpButton/HelpButton.tsx

+    const { runFirstValidSequence, closePrompts } = useActions(inAppPromptLogic)
+    const { isPromptVisible } = useValues(inAppPromptLogic)
+
+    console.log(isPromptVisible)


Reminder to remove this before merging

benjackwhite · 2022-07-26T06:51:51Z

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.scss

+        .LemonActionableTooltip__navigation--left {
+            margin-right: 0.25rem;
+        }
+        .LemonActionableTooltip__navigation--right {
+            margin-left: 0.25rem;
+        }


A simpler option without needing to have extra classnames would be

Suggested change

.LemonActionableTooltip__navigation--left {

margin-right: 0.25rem;

}

.LemonActionableTooltip__navigation--right {

margin-left: 0.25rem;

}

> * + * {

margin-left: 0.25rem;

}

This is what space-x does as a utility for example

benjackwhite · 2022-07-26T07:00:32Z

frontend/src/lib/logic/inAppPrompt/inAppPromptLogic.tsx

+            show.then(() => {
+                actions.updatePromptState({ step: values.currentStep })
+            }).catch((err) => console.error(err))


Not super strongly opinionated on this but if we are in an async function perhaps we should keep things async-style and try-catch this?

benjackwhite · 2022-07-26T07:03:36Z

frontend/src/lib/logic/inAppPrompt/inAppPromptLogic.tsx

+        [router.actions.locationChanged]: () => {
+            actions.closePrompts()
+            actions.findValidSequences()
+        },


There's also urlToAction from kea-router which could listen to * route and trigger changes that way?

benjackwhite · 2022-07-26T07:06:41Z

frontend/src/lib/logic/inAppPrompt/inAppPromptLogic.tsx

+    events(({ actions, cache }) => ({
+        afterMount: () => {
+            posthog.onFeatureFlags(async (_, variants) => {
+                if (variants[FEATURE_FLAGS.IN_APP_PROMPTS_EXPERIMENT] === 'test') {
+                    actions.syncState({ forceRun: true })
+                }
+            })
+        },
+        beforeUnmount: [
+            () => {
+                cache.runOnClose?.()
+            },
+        ],
+    })),


I believe these have their own top-level builders so you could write it:

Suggested change

events(({ actions, cache }) => ({

afterMount: () => {

posthog.onFeatureFlags(async (_, variants) => {

if (variants[FEATURE_FLAGS.IN_APP_PROMPTS_EXPERIMENT] === 'test') {

actions.syncState({ forceRun: true })

}

})

},

beforeUnmount: [

() => {

cache.runOnClose?.()

},

],

})),

afterMount(({ actions }) => {

posthog.onFeatureFlags(async (_, variants) => {

if (variants[FEATURE_FLAGS.IN_APP_PROMPTS_EXPERIMENT] === 'test') {

actions.syncState({ forceRun: true })

}

})

}),

beforeUnmount(({ cache }) => cache.runOnClose?.()),

benjackwhite · 2022-07-26T07:07:59Z

frontend/src/scenes/dashboard/DashboardHeader.tsx

@@ -239,6 +239,7 @@ export function DashboardHeader(): JSX.Element | null {
                                    <LemonButton
                                        type="secondary"
                                        data-attr="dashboard-share-button"
+                                        data-tooltip="experiment-new-dashboard-product-tour-1"


Thinking to the future... We use data-attr everywhere and encourage people to do so for autocapture. Might be good to have that as a selectable option (thinking more about productizing the feature)

benjackwhite · 2022-07-26T07:09:47Z

frontend/src/scenes/dashboard/Dashboards.tsx

+                            closePrompts()
+                            showNewDashboardModal()


This is gonna be fun... We'll have to see how it goes with the trial but perhaps it shouldn't be possible to do anything other than interact with the tooltips as there are lots of other places that create modals...

benjackwhite · 2022-07-26T07:17:00Z

posthog/api/prompt.py

+        if states_to_create:
+            PromptSequenceState.objects.bulk_create(states_to_create)
+        if states_to_update:
+            PromptSequenceState.objects.bulk_update(
+                states_to_update, ["last_updated_at", "step", "completed", "dismissed"]
+            )


Totally fine for now but if we want to productionize this it's probably worth having this wrapped in a transaction, otherwise we could get into a weird state where half of your request worked. But probably overall would have a different implementation anyways so all good :D

joethreepwood · 2022-07-26T09:37:07Z

...however I feel like we need to make each sequence of tooltips independent of the navigation order a certain user might follow. We can't enforce users to discover the app in a specific way, so we should just create standalone product tours for each page, regardless of the direction they take

Happy to draft up some copy for this and tooltip text v2 is on my current sprint...

However, I do think it could be worth revisiting the idea of a multi-page tutorial flow in the future, even if we just test it down the line. The main reason for this is that it'd give a chance to provide better context of how some tools can be used together contextually -- such as insights and dashboards, persons and session recordings, etc. I also think that, if we're confident what a majority of people want to achieve and can bake that into the flow, it could lead to the best result.

Not a strong feeling for now, just something I'd like to consider for the future.

joethreepwood · 2022-07-26T10:02:36Z

The current flow is based on this copy...

...however I feel like we need to make each sequence of tooltips independent of the navigation order a certain user might follow. We can't enforce users to discover the app in a specific way, so we should just create standalone product tours for each page, regardless of the direction they take

Also, to confirm: If the tutorial flow is going to be independant then will skipping the tutorial skip the ENTIRE tutorial, or just the content for a single page?

kappa90 · 2022-07-26T10:35:25Z

...however I feel like we need to make each sequence of tooltips independent of the navigation order a certain user might follow. We can't enforce users to discover the app in a specific way, so we should just create standalone product tours for each page, regardless of the direction they take

Happy to draft up some copy for this and tooltip text v2 is on my current sprint...

However, I do think it could be worth revisiting the idea of a multi-page tutorial flow in the future, even if we just test it down the line. The main reason for this is that it'd give a chance to provide better context of how some tools can be used together contextually -- such as insights and dashboards, persons and session recordings, etc. I also think that, if we're confident what a majority of people want to achieve and can bake that into the flow, it could lead to the best result.

Not a strong feeling for now, just something I'd like to consider for the future.

I think we just need to slightly tweak the copy so that one doesn't feel like they jumped in the middle of something.

From my experience, unless you force a flow, it's nearly impossible to dictate what the user does.

Maybe a solution to this is to have a check-list of product-tours the user can do, and have them complete them all. That way they can do them in whatever order, and we keep the copy focused on the current page.

Also, to confirm: If the tutorial flow is going to be independant then will skipping the tutorial skip the ENTIRE tutorial, or just the content for a single page?

This would disable product tours in general, which can be then re-triggered from the Help Menu.

joethreepwood · 2022-07-26T17:25:27Z

For now, I've written up a draft of copy that gives 1-3 tooltip prompts for every main page in the app. It's a bit rough, with a focus on highlighting main functionality and features that may be missed (such as live events toggle).

Here it is.

pauldambra

First pass over, looks good. Some nice use of TS types 💖

Needs API tests and updating from master (but you know that :))

pauldambra · 2022-07-27T09:52:08Z

frontend/src/lib/constants.tsx

@@ -117,6 +117,7 @@ export const FEATURE_FLAGS = {
    TOOLBAR_LAUNCH_SIDE_ACTION: 'toolbar-launch-side-action', // owner: @pauldambra,
    FEATURE_FLAG_EXPERIENCE_CONTINUITY: 'feature-flag-experience-continuity', // owner: @neilkakkar
    ASYNC_EXPORT_CSV_FOR_LIVE_EVENTS: 'ASYNC_EXPORT_CSV_FOR_LIVE_EVENTS', // owner: @pauldambra
+    IN_APP_PROMPTS_EXPERIMENT: 'IN_APP_PROMPTS_EXPERIMENT', // owner: @kappa90


definitely need to update from master... there should be conflicts here I think

pauldambra · 2022-07-27T09:52:40Z

frontend/src/lib/logic/inAppPrompt/inAppPromptEventCaptureLogic.ts

+import type { inAppPromptEventCaptureLogicType } from './inAppPromptEventCaptureLogicType'
+import posthog from 'posthog-js'
+
+const inAppPromptEventCaptureLogic = kea<inAppPromptEventCaptureLogicType>([


🙌 Yes, to not bloating the existing event capture logic!

pauldambra · 2022-07-27T11:18:47Z

frontend/src/lib/logic/inAppPrompt/inAppPromptLogic.tsx

+    icon?: string
+}
+
+export type Tooltip = Prompt & { type: 'tooltip' }


pauldambra · 2022-07-27T11:26:48Z

latest_migrations.manifest

@@ -3,7 +3,7 @@ auth: 0012_alter_user_first_name_max_length
 axes: 0006_remove_accesslog_trusted
 contenttypes: 0002_remove_content_type_name
 ee: 0013_silence_deprecated_tags_warnings
-posthog: 0251_event_buffer
+posthog: 0252_auto_20220721_0858


this will clash as well. I snuck in 252 ahead of you :)

And we should be naming the migrations when creating them

pauldambra · 2022-07-27T11:28:29Z

posthog/api/prompt.py

+
+    @action(methods=["PATCH"], detail=False)
+    def my_prompts(self, request: request.Request, **kwargs):
+        if not request.user.is_authenticated:  # for mypy


Can use a test to prove it but I'd expect this to be handled by middleware not each route

pauldambra · 2022-07-27T11:34:19Z

posthog/models/prompt.py

+
+# this model is currently unused as we are running an experimentation config, might be used in the future
+# or completely removed if we allow feature flags to carry a payload, which would be a prompts configuration JSON
+class PromptSequence(models.Model):


We can't delete models/fields for various reasons. If we're not using this we should remove it and only add it when we need it. Otherwise there's a risk it lives on in the code unused for a long time

kappa90 · 2022-07-29T20:29:21Z

@benjackwhite @pauldambra introduced all your feedback, we can merge Monday morning :)

benjackwhite

No big complaints only a few comments 👍

benjackwhite · 2022-08-01T08:32:45Z

frontend/src/scenes/project-homepage/ProjectHomepage.tsx

@@ -43,7 +44,7 @@ export function ProjectHomepage(): JSX.Element {
                Invite members
            </LemonButton>
            <LemonButton
-                data-attr="project-home-new-insight"


I wonder if this is used anywhere in autocapture analytics....

This is definitely a mistake on my side, thanks for spotting that!

benjackwhite · 2022-08-01T08:33:07Z

frontend/src/scenes/session-recordings/player/PlayerEvents.tsx

@@ -118,6 +118,7 @@ export function PlayerEvents(): JSX.Element {
                    onClick={() => {
                        handleEventClick(event.playerPosition)
                    }}
+                    data-tooltip={'recording-event-list'}


nit: doesn't need to be bracketed

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.scss

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.tsx

frontend/src/lib/logic/inAppPrompt/inAppPromptLogic.test.ts

posthog/api/prompt.py

posthog/api/test/test_prompt.py

Twixes

AntD alert

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.tsx

fixes implemented

Twixes · 2022-08-01T09:24:13Z

posthog/models/prompt.py

+        constraints = [
+            models.UniqueConstraint(fields=["team", "person", "key"], name="unique sequence key for person for team")
+        ]
+
+    team: models.ForeignKey = models.ForeignKey("Team", on_delete=models.CASCADE)
+    person: models.ForeignKey = models.ForeignKey("Person", on_delete=models.CASCADE)


As a user, I don't think it'd make sense for me to go through a flow multiple times just because I have multiple projects. Why not key this just ["person", "key"]?

Twixes · 2022-08-01T09:27:06Z

posthog/api/prompt.py

+        )
+
+        if not person_id:
+            return Response("", status=status.HTTP_404_NOT_FOUND)


This won't have a JSON payload, I believe we should raise a exceptions.NotFound() (not sure actually, since this is an @action, but we definitely should return the standard 404 JSON response)

Twixes · 2022-08-01T09:27:55Z

posthog/api/prompt.py

+
+        for sequence in all_sequences:
+            local_state = next((s for s in local_states if sequence["key"] == s["key"]), None)
+            saved_state: Union[PromptSequenceState, None] = next(


Tip: Union[T, None] can be represented as Optional[T] ;)

Twixes · 2022-08-01T09:31:41Z

posthog/api/prompt.py

+class PromptSequenceStateSerializer(serializers.HyperlinkedModelSerializer):
+    class Meta:
+        model = PromptSequenceState
+        fields = ["key", "last_updated_at", "step", "completed", "dismissed"]


Should they all also be marked as read_only_fields?

Twixes · 2022-08-01T09:34:14Z

posthog/api/__init__.py

@@ -57,6 +58,7 @@ def api_not_found(request):
 )
 projects_router.register(r"annotations", annotation.AnnotationsViewSet, "project_annotations", ["team_id"])
 projects_router.register(r"feature_flags", feature_flag.FeatureFlagViewSet, "project_feature_flags", ["team_id"])
+projects_router.register(r"prompts", prompt.PromptSequenceStateViewSet, "project_feature_flags", ["team_id"])


basename needs to be updated

Suggested change

projects_router.register(r"prompts", prompt.PromptSequenceStateViewSet, "project_feature_flags", ["team_id"])

projects_router.register(r"prompts", prompt.PromptSequenceStateViewSet, "project_prompts", ["team_id"])

As for the shape of the API though, wouldn't it be more intuitive for prompts to be scoped per user instead per project? (mentioned this in a comment above too)

Twixes · 2022-08-01T09:35:51Z

frontend/src/scenes/annotations/index.tsx

+                        data-attr="create-annotation"
+                        data-tooltip="annotations-new-button"


Could we just reuse data-attr instead of adding data-tooltip? 🤔 Seems like they serve a very similar purpose

Twixes · 2022-08-01T09:36:47Z

frontend/src/lib/constants.tsx

@@ -118,6 +118,7 @@ export const FEATURE_FLAGS = {
    // Re-enable person modal CSV downloads when frontend can support new entity properties
    PERSON_MODAL_EXPORTS: 'person-modal-exports', // hot potato see https://github.com/PostHog/posthog/pull/10824
    BILLING_LOCK_EVERYTHING: 'billing-lock-everything', // owner @timgl
+    IN_APP_PROMPTS_EXPERIMENT: 'IN_APP_PROMPTS_EXPERIMENT', // owner: @kappa90


Absolute nit… but could we keep the same kebab-case convention for feature flag keys?

Twixes · 2022-08-01T09:38:56Z

frontend/src/lib/components/icons.tsx

+
+export function IconMessages(props: React.SVGProps<SVGSVGElement>): JSX.Element {
+    return (
+        <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" {...props}>


This doesn't seem right – all our icons should be using a 24x24 viewbox.

Twixes · 2022-08-01T09:39:35Z

frontend/src/lib/components/icons.tsx

+        <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" {...props}>
+            <path
+                d="M13 2V9H3.17L2 10.17V2H13ZM14 0H1C0.45 0 0 0.45 0 1V15L4 11H14C14.55 11 15 10.55 15 10V1C15 0.45 14.55 0 14 0ZM19 4H17V13H4V15C4 15.55 4.45 16 5 16H16L20 20V5C20 4.45 19.55 4 19 4Z"
+                fill="#747EA2"


Icons needs to have fill="currentColor" so that they can adjust to button colors

kappa90 added 7 commits July 12, 2022 15:40

frontend-y tooltips almost done

ecd619b

clean up

121487d

Merge branch 'master' into feat/in-app-prompts

cb68fd0

wip prompts

6d57c9e

prompts refactor

ab6437b

persist state in cache

a35ba00

add comments

540f664

kappa90 requested review from clarkus, pauldambra, benjackwhite and joethreepwood July 25, 2022 15:48

benjackwhite reviewed Jul 26, 2022

View reviewed changes

jest tests

7feebe5

pauldambra reviewed Jul 27, 2022

View reviewed changes

kappa90 added 6 commits July 28, 2022 11:34

api test

96c72c7

new tooltips

f5e3dca

last touches

bcaac41

remove old migration

5e396f7

Merge remote-tracking branch 'origin/master' into feat/in-app-prompts

ba0c52d

fixes after merge

7669851

kappa90 marked this pull request as ready for review July 29, 2022 19:29

kappa90 added 3 commits July 29, 2022 20:44

mypy types

364ac16

mypy types

7fdc50a

mypy types

197022c

kappa90 requested a review from pauldambra July 29, 2022 20:28

kappa90 requested a review from benjackwhite July 29, 2022 20:28

benjackwhite approved these changes Aug 1, 2022

View reviewed changes

small nit fixes

5ed4c93

kappa90 enabled auto-merge (squash) August 1, 2022 08:41

pauldambra previously requested changes Aug 1, 2022

View reviewed changes

add test

16edb90

kappa90 requested a review from pauldambra August 1, 2022 09:20

Twixes previously requested changes Aug 1, 2022

View reviewed changes

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.tsx Outdated Show resolved Hide resolved

frontend/src/lib/components/LemonActionableTooltip/LemonActionableTooltip.tsx Outdated Show resolved Hide resolved

removed antd icons

7d8b8f9

kappa90 requested a review from Twixes August 1, 2022 09:30

kappa90 merged commit 28ae03d into master Aug 1, 2022

kappa90 deleted the feat/in-app-prompts branch August 1, 2022 09:47

Twixes reviewed Aug 1, 2022

View reviewed changes

kappa90 restored the feat/in-app-prompts branch August 1, 2022 09:54

kappa90 mentioned this pull request Aug 1, 2022

fix: in app prompts #11061

Merged

kappa90 deleted the feat/in-app-prompts branch August 1, 2022 10:54

This was referenced Aug 1, 2022

chore: show prompts to new users #11072

Merged

In-App Onboarding/Guides/Wizard #4879

Open

kappa90 mentioned this pull request Aug 31, 2022

Sprint 5th September - 16th september #11579

Closed

	projects_router.register(r"prompts", prompt.PromptSequenceStateViewSet, "project_feature_flags", ["team_id"])
	projects_router.register(r"prompts", prompt.PromptSequenceStateViewSet, "project_prompts", ["team_id"])

		data-attr="create-annotation"
		data-tooltip="annotations-new-button"

feat: in app prompts experiment #10956

feat: in app prompts experiment #10956

Conversation

kappa90 commented Jul 25, 2022 • edited Loading

Problem

Changes

How did you test this code?

joesaunderson commented Jul 25, 2022

benjackwhite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joethreepwood commented Jul 26, 2022

joethreepwood commented Jul 26, 2022

kappa90 commented Jul 26, 2022

joethreepwood commented Jul 26, 2022

pauldambra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kappa90 commented Jul 29, 2022

benjackwhite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Twixes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kappa90 commented Jul 25, 2022 •

edited

Loading