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

fix(darkmode): prefer bg-light over bg-white #27425

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

Conversation

kevin-secrist
Copy link
Contributor

@kevin-secrist kevin-secrist commented Jan 9, 2025

Hello! Having a little bit of fun squashing bugs while also learning a little bit about the repo and PostHog in general. I love seeing this dev out in the open!

Problem

I had a UX issue during onboarding that made one of the components difficult to use. After a little tinkering it looks like an issue with dark mode. I've replaced all instances of .bg-white with .bg-light where it makes sense (sorry if I'm overdoing it here). Some places (like the MFA QR code) are meant to be white and are left unchanged, but a few others I'm guessing should just be the normal background color. Let me know if there's a different class that's more appropriate.

Changes

Onboarding Dashboard Template Configuration

AnFsMhizli
brave_YEFW3Yq6j7

Debug Repl

brave_CFAjcp5Evd
szIJTNhX9C

Shared Metric Experiment

In this case, it's not a dark mode issue but a consistency issue. It previously used .bg-light for specifically for dark mode but .bg-white for light mode.

9mJJbpKElV
brave_H5gzQxceDk

Heatmap iFrame

I have not tested this, but my assumption is that this white background will become bg-light instead.

Please ignore flaming spiderhog with tophat.

Screenshot 2025-01-09 182346

Onboarding iFrame

Same deal as above, this one is accessible at this url https://us.posthog.com/project/<projectId>/onboarding/product_analytics?step=dashboard_template_configure

image

Does this work well for both Cloud and self-hosted?

Should have no impact

How did you test this code?

This was all done in chrome console, toggling classes.

@rafaeelaudibert
Copy link
Member

Assigning to you @adamleithp because you've been working with our colors :)

@adamleithp
Copy link
Contributor

I'll have a look at this this week!

@adamleithp adamleithp self-assigned this Jan 16, 2025
@adamleithp
Copy link
Contributor

Hey @kevin-secrist I'm struggling how to replicate these issues/ scenarios. I've reached out to some other devs and will return to this when I get the chance.

@adamleithp adamleithp added the ux changes concerning the users experience label Jan 16, 2025
@kevin-secrist
Copy link
Contributor Author

kevin-secrist commented Jan 16, 2025

Hi Adam, yes I'm actually having some issues replicating these scenarios now. For onboarding in particular it looks like there's some conditional logic here that I haven't dug into enough to fully understand. The mentioned featureFlag is set to test, and I've ensured my window size is big enough but I don't know what that last combinedSnippetAndLiveEventsHosts bit is for. I don't see anything immediately obvious in recently merged PRs that would change this behavior.

const showTemplateSteps =
featureFlags[FEATURE_FLAGS.ONBOARDING_DASHBOARD_TEMPLATES] == 'test' &&
window.innerWidth > 1000 &&
combinedSnippetAndLiveEventsHosts.length > 0

{/* this is two conditionals because they need to be direct children of the wrapper */}
{showTemplateSteps ? (
<OnboardingDashboardTemplateSelectStep stepKey={OnboardingStepKey.DASHBOARD_TEMPLATE} />
) : null}
{showTemplateSteps ? (
<OnboardingDashboardTemplateConfigureStep stepKey={OnboardingStepKey.DASHBOARD_TEMPLATE_CONFIGURE} />
) : null}

Here are direct URLs to the others that I can still replicate:

  • Shared metrics: https://us.posthog.com/project/<projectId>/experiments/shared-metrics/new
  • Hog Repl: https://us.posthog.com/project/<projectId>/debug/hog
  • Heatmap: https://us.posthog.com/project/<projectId>/heatmaps

All the onboarding stuff was previously available at https://us.posthog.com/project/<projectId>/onboarding/product_analytics?step=dashboard_template_configure but is not bringing me to that page anymore.

I'll also fix the merge conflicts now

@kevin-secrist
Copy link
Contributor Author

kevin-secrist commented Jan 16, 2025

Oh, figured it out - the logged in user needs to have recent/active traffic/analytics coming in. If you use a demo site that isn't getting traffic (like I am) then you won't see the dashboard template. Once I went to my site and clicked around a bit and did a hard refresh, I was able to see the page. However, direct link does not work:

Go to the onboarding wiz: https://us.posthog.com/project/<projectId>/onboarding/product_analytics, then in the breadcrumb there should be Dashboard Template you can click to skip straight to it

image

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale ux changes concerning the users experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants