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

feat(surveys): Custom and tab widget #19223

Merged
merged 12 commits into from
Dec 13, 2023
Merged

feat(surveys): Custom and tab widget #19223

merged 12 commits into from
Dec 13, 2023

Conversation

liyiy
Copy link
Contributor

@liyiy liyiy commented Dec 9, 2023

Problem

Changes

to go in with PostHog/posthog-js#933

Allow users to set up their own buttons for surveys + extra tab widget option

Screen.Recording.2023-12-08.at.7.57.30.PM.mov

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

Copy link
Contributor

github-actions bot commented Dec 9, 2023

Size Change: +160 B (0%)

Total Size: 1.83 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.83 MB +160 B (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

@jurajmajerik jurajmajerik left a comment

Choose a reason for hiding this comment

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

LGTM!

@neilkakkar
Copy link
Collaborator

It's a bit confusing to me how widget & popover are both presentation types, but then the widget also creates a popover. If we then have a modal instead of the popover, the widget should also support opening as a modal?

Maybe @annikaschmid has ideas on where this makes more sense to go.

Also, how does targeting work with the widget? Doesn't make sense to have both, targeting and the widget section, since I assume if someone chooses the widget they don't want it to randomly popup like the regular popover.

Makes me think the widget is more of a customisation option.

@liyiy
Copy link
Contributor Author

liyiy commented Dec 11, 2023

It's a bit confusing to me how widget & popover are both presentation types, but then the widget also creates a popover. If we then have a modal instead of the popover, the widget should also support opening as a modal?

Maybe @annikaschmid has ideas on where this makes more sense to go.

Also, how does targeting work with the widget? Doesn't make sense to have both, targeting and the widget section, since I assume if someone chooses the widget they don't want it to randomly popup like the regular popover.

Makes me think the widget is more of a customisation option.

Widget made the most sense as a higher level customization type because while it does use the normal "surveys popup", it's definitely not a question type, that would get super confusing very quickly.

Targeting would still work similarly with the widget, though the survey wait period options etc won't apply, so I can remove that, but for the other options it still would. Why can't you have a widget that shows up on specific pages if a user wants that?

FYI this is all feature flagged, so happy to make edits after too if they're not urgent for merge, so that it's easier to give feedback with the feature in live for just us

@annikaschmid
Copy link

I have some design feedback here:

  1. I think the name "widget" doesn't work. A "widget" is quite a general term, and might as well also describe the popover. I had a look at some standard terminology, and what we call "widget", is usually called "button"/"feedback button", with another option being "embedded" (which we don't need to over just yet). And then our popover others simply call "survey".
    I think we should call it "Button" or "Feedback button" with the description "Side-of-the-page Button that opens the popover"

  2. We shouldn't have an accordion option "Widget". What is in there are the styling options of the widget, so it should be available in 'Customisation'. The current styling of the button also looks a lot less refined than our survey popover. By default, we should have the same "Background colour" and "Border colour" as the popover. IMO it's even optional to offer a separate styling option.
    If we want to do that, the 'Customisation' tab should have two headlines to divide the content: "Popover customisation" and "Feedback button customisation"

  3. For visual cohesiveness, the feedback button border radius should be the same as the survey popover

  4. I wish we would do something more clever with the interaction of the button and the popover, rather than just opening the popover next to the button. Right now it's a bit basic, and the popover just hangs in the air. For inspiration, you can look at our own 3000 sidebar for docs and feedback, or even how Hotjar do it.

  5. The popover currently has the 'x' to close between the popover and the button and also closes via the button. I don't think this looks good. If we do something clever in 4), this point of feedback will be redundant


Before we roll this out, can we make it available behind a flag for the PostHog team first and collect some more feedback please? I don't think this feature is just there yet.

@liyiy
Copy link
Contributor Author

liyiy commented Dec 12, 2023

@annikaschmid

  1. Yep I'm fine with renaming it to Button but as I looked around it seemed like Widget was how it's more commonly referred to as in the industry? Button also seems a bit vague here? Like on the survey there are options for "button text", etc but nothing else is referred to as widget. Thoughts on that?

  2. Can do with moving the customization for widget into the customization panel. However I don't think we should focus on it being named as a feedback button as much because I think the customizable button part is the selling point. "Popover customization" and "Widget customization" seems more obvious here than "Popover customization" and "Button customization"

  3. Sure

  4. I'm okay with punting on this for the next PR to address

  5. Happy to loop in @corywatilo 's input here for that

Before we roll this out, can we make it available behind a flag for the PostHog team first and collect some more feedback please?

Yeah I already have it set up behind a flag and I put the word beta all around it. There are users already asking for this so I think they'd be happy to try it out even if it's not fully polished yet :') Maybe we just don't advertise it as a complete feature then

@liyiy liyiy merged commit 255f6b1 into master Dec 13, 2023
79 checks passed
@liyiy liyiy deleted the surveys-widget branch December 13, 2023 00:38
@annikaschmid
Copy link

  1. When you looked at the industry, was 'widget' specifically used for the sidebar feedback button, or the survey popover in general? Because just from the word "popover" vs "widget", I think it would be hard for our users to know what is what. Even we called the feedback widget "widget" before it was called "popover". What if we call it "Feedback button"? A quick Google search confirms that this is what we are doing. I know people can rename it to "not feedback", but based on the functionality, it would still be used for some sort of feedback use case, since it opens a survey, which is gathering information. If we find a lot of people use it for other use cases, we can always rename it.

I am open to other suggestions as well, but I think "widget" it too wide of a term and doesn't differentiate this enough from the popover.
Screenshot 2023-12-13 at 17 42 49

Screenshot 2023-12-13 at 17 43 32
  1. How would you do it? I think it's a small interaction where we not necessarily need Cory's help, since we also have 2 examples already. e.g. the button is visually selected, and if you click it again, the popover closes. Or the 'x' to close is on the left side of the popover? That could work as well

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.

5 participants