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

fix: remove drag button from not-draft surveys #27565

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

lucasheriques
Copy link
Contributor

Problem

we currently store questions and surveys as a JSON field, and the question do not have any ids.

this means that, if we change the order of questions in a survey AFTER its live, the order will not be correct.

The ideal solution here is to change the survey.questions model directly, and add an id to each question, instead of relying on the id.

However, as a band-aid fix for now, we're just not allowing users to reorder question after a survey is launched.

Related issue: #27200

Changes

No visible changes, but the draggable icon next to the question is no longer visible for a survey that is launched/ended.

CleanShot 2025-01-15 at 16 58 53@2x
Non-draft survey

CleanShot 2025-01-15 at 16 59 09@2x
Draft survey

👉 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?

  • Unit tests on the new component
  • Verified locally the behavior is what we expect

@lucasheriques lucasheriques requested review from dmarticus and a team January 15, 2025 20:02
Copy link
Contributor

github-actions bot commented Jan 15, 2025

Size Change: 0 B

Total Size: 1.13 MB

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

compressed-size-action

@marandaneto
Copy link
Member

@lucasheriques can we fix the spacing when the drag icon isn't visible? you can see in screenshot 1 an extra space/padding between questions 1 and 2 in relation to the confirmation message.

Copy link
Member

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

left a comment otherwise LGTM.
I agree with the quick bandaid solution until we come up with a proper fix.

@lucasheriques lucasheriques enabled auto-merge (squash) January 16, 2025 18:30
@lucasheriques lucasheriques merged commit 5469150 into master Jan 16, 2025
98 checks passed
@lucasheriques lucasheriques deleted the fix/remove-drag-button-from-non-draft-surveys branch January 16, 2025 18:53
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.

2 participants