Skip to content

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Jan 16, 2023

Changes

This is part 1 of probably 3 PRs replacing Ant icons with Lemon ones. Only targeting isolated cases of those icons here – those embedded in other Ant components such as Select will be replaced when those components are refactored away. Also skipping the Toolbar here (leaving that for the third PR in the series).

This also reduces usage of two components targeted for removal: InlineMessage (redundant because of AlertMessage) and PayCard (redundant because of PayGateMini). Completely removes two completely redundant components: SelectDownIcon and CohortIcon.

How did you test this code?

I reviewed every single case manually, but we don't have visual regression tests for almost any of them, and it'd be a massive effort to add them in this PR, so take a look locally too.

@Twixes Twixes requested a review from pauldambra January 16, 2023 15:14
@Twixes
Copy link
Member Author

Twixes commented Jan 16, 2023

Hmm, before this PR we weren't actually showing pay gates for correlation analysis upselling, due to what looks like a bug (line of code). So users on free plans weren't informed such a feature exists. @neilkakkar Can you confirm whether this visual diff (with a pay gate once again) look right to you: https://github.com/PostHog/posthog/pull/13738/files#diff-5a0b5d67b76751e047e19279eddcf830288393eb8a54e8d0a4db1aaed880915b?

@Twixes Twixes requested a review from neilkakkar January 16, 2023 15:40
@neilkakkar
Copy link
Contributor

Thanks for catching, yep looks good!

@Twixes Twixes removed the request for review from neilkakkar January 16, 2023 15:52
@Twixes Twixes mentioned this pull request Jan 16, 2023
59 tasks
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

looks good... although I'm reviewing this at the end of the day so you could have hidden anything in here 🤣

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.

3 participants