Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Removing target options for survey resets its value to remove validation error #27139

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lucasheriques
Copy link
Contributor

@lucasheriques lucasheriques commented Dec 23, 2024

Problem

Fixes #22186

When we add property targeting to a surveys and then remove it, we can't save the survey anymore.

Because there's a hidden form validation error hidden in the UI.

To fix it:

Changes

CleanShot 2024-12-23 at 15 23 18@2x

Old deleting behavior: https://www.loom.com/share/7e84a69cb97c4966970b2a796e375826

New behavior:

CleanShot.2024-12-23.at.15.24.05.mp4

Notice that now, after removing the property, there's no hidden form validation error.

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

Does this work well for both Cloud and self-hosted?

How did you test this code?

@lucasheriques lucasheriques requested a review from a team December 23, 2024 18:24
Copy link
Contributor

github-actions bot commented Dec 23, 2024

Size Change: 0 B

Total Size: 1.11 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.11 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Surveys: Can't remove user targeting options without setting rollout
2 participants