-
Notifications
You must be signed in to change notification settings - Fork 138
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(surveys): polishing the popup survey UI #1279
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size Change: +34.5 kB (+3.1%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
3ba700a
to
d203f58
Compare
.survey-box:has(.survey-question:empty):not(:has(.description)) textarea { | ||
margin-top: 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only want to remove the margin in the event where there's no description (the selector is gone) AND there's no question (the selector is empty). Otherwise, the margins should remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the :has pseudo-selector ... supported in all major browsers since 2023, but is this a risky change still?
This is fine in my opinion. It's a small styling detail that just won't work in a tiny minority of cases.
This is more like a feature request than a bug - we currently don't allow customization of the confirmation message text. Feel free to address in a follow up :) |
@jurajmajerik yeah, I haven't finished part 4 yet, sorry! I thought I'd gotten it working but it's not ready. |
There's indeed something going wrong with my |
Changes
Closes issues 1,3, and 4 from this ticket PostHog/posthog#20851.
Fixing 4 requires some changes to posthog, too: PostHog/posthog#23460
Issue 2 I wasn't able to reproduce; I need more context from @annikaschmid and/or @jurajmajerik
Quick note on browser compatibility: I'm using the
:has
pseudo-selector for determining whether to apply margins or not. Per caniuse, this has been supported in all major browsers since 2023, but is this a risky change still?Visual Diff For Part 1
Before
After
Visual Diff for Part 3
Before
After
Visual Diff for Part 4
Before
After
...
Checklist