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.

Sign up for GitHub

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

Jump to bottom

chore(3000): Fix dark mode colors #16108

Merged
merged 18 commits into from
Jun 23, 2023
Merged

chore(3000): Fix dark mode colors #16108

merged 18 commits into from
Jun 23, 2023

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Jun 16, 2023

Changes

Adding some missing 3000 color variants for buttons, making dark mode polished.

@Twixes Twixes force-pushed the 3000-colors branch 7 times, most recently from b5f3a5d to fe02c2a Compare June 22, 2023 10:15
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Twixes
Twixes commented Jun 22, 2023
View reviewed changes
frontend/__snapshots__/components-html-elements-display--editable-display-with-preselection.png
Copy link
Member Author

Choose a reason for hiding this comment

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

I had no idea these were clickable, had to change this since the extralight color is gone now, but I think using the primary blue is also an improvement in terms of affordance

@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@PostHog PostHog deleted a comment from posthog-bot Jun 22, 2023
@Twixes Twixes marked this pull request as ready for review June 22, 2023 16:15
@Twixes
Copy link
Member Author

Twixes commented Jun 22, 2023

I think this fixes basically all dark mode issues, besides some very minor things maybe. Do click through it and take a look at the UI snapshot changes though.

@Twixes Twixes requested a review from liyiy June 22, 2023 17:28
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Twixes
Twixes commented Jun 23, 2023
View reviewed changes
frontend/__snapshots__/components-cards-insight-card--insight-card.png
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed bold number here since it's been flaky on many PRs

benjackwhite
benjackwhite approved these changes Jun 23, 2023
View reviewed changes
Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

Looked through as much of the UI locally as I could and didn't notice anything off so 👍

@thmsobrmlr
Copy link
Contributor

Woah, nice!

Going to leave a couple things that caught my attention here, definitely understand if that's not in scope for now.

  1. Contrast between primary blue and background is too low
blue_on_background Screenshot 2023-06-23 at 12 58 06
  1. Visual hierachy between title and description in insight card gets lost. Somehow this is not a problem in light mode.
insight_card_visual_hierarchy
  1. Colors of property icons are kind of dull
property_icons_dull

@Twixes Twixes merged commit fa516b1 into master Jun 23, 2023
@Twixes Twixes deleted the 3000-colors branch June 23, 2023 12:44
@Twixes Twixes mentioned this pull request Oct 19, 2023
Closed
41 tasks
@thmsobrmlr thmsobrmlr mentioned this pull request Aug 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjackwhite benjackwhite benjackwhite approved these changes

@corywatilo corywatilo Awaiting requested review from corywatilo

@liyiy liyiy Awaiting requested review from liyiy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Twixes @posthog-bot @thmsobrmlr @benjackwhite