-
Notifications
You must be signed in to change notification settings - Fork 216
feat: allow customizing text color on surveys #2787
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: allow customizing text color on surveys #2787
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
77ac12e to
f1a534d
Compare
|
Size Change: +715 B (+0.01%) Total Size: 5.3 MB
ℹ️ View Unchanged
|
f1a534d to
39a0b5d
Compare
39a0b5d to
8f9ca73
Compare
8f9ca73 to
3c98c33
Compare
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.
there are many tests here and some of them repeated, so i'm consolidating them a little
| // keep in sync with frontend/src/types.ts -> SurveyAppearance | ||
| backgroundColor?: string | ||
| // Optional override for main survey text color. If not set, auto-calculated from backgroundColor. | ||
| textColor?: string |
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.
this will now exist on the rn sdk types, so we should still document somewhere that it doesn't work on react-native (until we update it)
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.
the implementation is a part of the PR, not sure if you saw it before the update, can you double check?
| placeholder={appearance.placeholder} | ||
| placeholderTextColor={ | ||
| getContrastingTextColor(appearance.inputBackground) === 'black' | ||
| (appearance.inputTextColor ?? getContrastingTextColor(appearance.inputBackground)) === 'black' |
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.
This would be white if inputTextColor is anything other than black right? Doesn't seem right
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.
| Case | Result |
|---|---|
| No inputTextColor, light bg → 'black' → needs 'white' contrast | dark placeholder ✓ |
| No inputTextColor, dark bg → 'white' → needs 'black' contrast | light placeholder ✓ |
| inputTextColor = 'black' → needs 'white' contrast | dark placeholder ✓ |
| inputTextColor = '#000000' → needs 'white' contrast | dark placeholder ✓ (FIXED) |
fixed it now, see if the table above makes sense too
| export type SurveyAppearanceTheme = Omit< | ||
| Required<SurveyAppearance>, | ||
| 'widgetSelector' | 'widgetType' | 'widgetColor' | 'widgetLabel' | 'shuffleQuestions' | ||
| > | ||
| 'widgetSelector' | 'widgetType' | 'widgetColor' | 'widgetLabel' | 'shuffleQuestions' | 'textColor' | 'inputTextColor' | ||
| > & { | ||
| textColor?: string | ||
| inputTextColor?: string | ||
| } |
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.
Don't really know how to read this type. Are we omitting textColor, inputTextColor from original type and adding them back as optional properties?
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.
its omitting all hardcoded values and adding back the 2 as optional
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.
Don't really know how to read this type. Are we omitting textColor, inputTextColor from original type and adding them back as optional properties?
yep! we're making most fields required (so we always have defaults to render with), but textColor and inputTextColor stay optional since they're overrides - if not set, we auto-calculate them from the background for contrast.
3cc50e3 to
c8d1fe7
Compare
Merge activity
|
## Problem The survey appearance customization UI needs improvement to be more organized and user-friendly. The current layout doesn't group related options logically and lacks explicit text color controls. Requires PostHog/posthog-js#2787 to be in before this is ready to ship ## Changes - Reorganized the survey appearance options into logical subgroups (Survey background, Inputs and ratings, Submit button) - Added explicit text color controls with "auto-contrast" hints - Improved responsive layout with better grid structure (1 column on mobile, 3 columns on larger screens) - Updated the "inherit" font label to be more concise Before: ![CleanShot 2025-12-17 at [email protected]](https://app.graphite.com/user-attachments/assets/3efe948e-54bb-46c3-ada4-6a481f150001.jpg) After: ![CleanShot 2025-12-17 at [email protected]](https://app.graphite.com/user-attachments/assets/b072dabe-2984-4d57-812a-ed7c95b060d7.jpg) ## How did you test this code? Tested the UI in both mobile and desktop viewports to ensure the responsive layout works correctly. Verified that all color controls function properly and that the auto-contrast hint displays correctly. Confirmed that the new text color fields are properly saved and applied to surveys. ## Changelog Yes, feature completed
## Problem The survey appearance customization UI needs improvement to be more organized and user-friendly. The current layout doesn't group related options logically and lacks explicit text color controls. Requires PostHog/posthog-js#2787 to be in before this is ready to ship ## Changes - Reorganized the survey appearance options into logical subgroups (Survey background, Inputs and ratings, Submit button) - Added explicit text color controls with "auto-contrast" hints - Improved responsive layout with better grid structure (1 column on mobile, 3 columns on larger screens) - Updated the "inherit" font label to be more concise Before: ![CleanShot 2025-12-17 at [email protected]](https://app.graphite.com/user-attachments/assets/3efe948e-54bb-46c3-ada4-6a481f150001.jpg) After: ![CleanShot 2025-12-17 at [email protected]](https://app.graphite.com/user-attachments/assets/b072dabe-2984-4d57-812a-ed7c95b060d7.jpg) ## How did you test this code? Tested the UI in both mobile and desktop viewports to ensure the responsive layout works correctly. Verified that all color controls function properly and that the auto-contrast hint displays correctly. Confirmed that the new text color fields are properly saved and applied to surveys. ## Changelog Yes, feature completed

Problem
Survey appearance customization needs improvement to ensure consistent text contrast across different elements, particularly for rating buttons and text inputs.
Changes
Implemented the changes for both React Native and Web (posthog-js) and now began sharing more fields between them too.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file