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

Devex/tailwindv4 color replacements attempt 4 #28175

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

Conversation

adamleithp
Copy link
Contributor

Problem

Changes

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

adamleithp and others added 30 commits January 30, 2025 15:44
…ed survey preview being z-index 9999px by wrapping in .survey-view
…o have bg- in front to match others, added z-index var for resizer
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR implements a comprehensive color system update across the PostHog frontend, migrating to Tailwind CSS v4 with semantic color tokens.

  • Replaces legacy color variables (--bg-3000, --border, --muted) with semantic equivalents (--bg-primary, --border-primary, --text-secondary) in SCSS files
  • Updates Tailwind configuration with new color system plugins including @csstools/postcss-gamut-mapping
  • Migrates text color classes from text-muted/text-muted-alt to text-secondary for better semantic meaning
  • Standardizes background colors using bg-surface-primary instead of bg-bg-light across components
  • Removes utilities.scss in favor of Tailwind's built-in color utilities

458 file(s) reviewed, 13 comment(s)
Edit PR Review Bot Settings | Greptile

ignoreAtRules: ['theme', 'custom-variant'],
},
],
'scss/at-rule-no-unknown': null,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Potential conflict with line 43-48 which also configures at-rule handling - consider consolidating these rules

Comment on lines 78 to 80
&:hover {
background-color: var(--accent-3000);
background-color: var(--bg-surface-primary);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Counter step hover now uses --bg-surface-primary instead of --accent-3000, which may affect visual hierarchy

frontend/src/lib/lemon-ui/colors.stories.tsx Show resolved Hide resolved
frontend/src/lib/ui/Colors/Colors.stories.tsx Show resolved Hide resolved
frontend/src/styles/mixins.scss Show resolved Hide resolved
frontend/src/toolbar/bar/Toolbar.scss Show resolved Hide resolved
webpack.config.js Show resolved Hide resolved
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

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