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): make link survey link optional #938

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Conversation

liyiy
Copy link
Contributor

@liyiy liyiy commented Dec 14, 2023

Changes

PostHog/posthog#19320

Link is now optional for link type surveys

Checklist

@liyiy liyiy added the bump minor Bump minor version when this PR gets merged label Dec 14, 2023
@liyiy liyiy requested a review from jurajmajerik December 14, 2023 01:23
Copy link

Size Change: 0 B

Total Size: 752 kB

ℹ️ View Unchanged
Filename Size
dist/array.full.js 177 kB
dist/array.js 118 kB
dist/es.js 118 kB
dist/exception-autocapture.js 12 kB
dist/module.js 118 kB
dist/recorder-v2.js 104 kB
dist/recorder.js 58.4 kB
dist/surveys.js 46.8 kB

compressed-size-action

@@ -587,8 +587,8 @@ export const createOpenTextOrLinkPopup = (
[`$survey_responded/${survey.id}`]: true,
},
})
if (surveyQuestionType === 'link') {
window.open(question.link || undefined)
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, why did we do it like this previously? window.open(undefined) opens an empty tab.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we originally required link urls for link type surveys so the button was always going to open something one way or another 😅

I think window.open("") would open something weird so passing in undefined was better

Copy link
Contributor

Choose a reason for hiding this comment

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

What does the CTA do now, if it doesn't link to something? 🤔

@liyiy liyiy merged commit 3e13700 into master Dec 14, 2023
14 checks passed
@liyiy liyiy deleted the surveys-link-optional branch December 14, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants