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

Added Support for Dark Mode ๐ŸŒ™ #1015

Merged
merged 10 commits into from
Nov 1, 2023
Merged

Added Support for Dark Mode ๐ŸŒ™ #1015

merged 10 commits into from
Nov 1, 2023

Conversation

MathurAditya724
Copy link
Contributor

Issue - #1006

  • Added CSS Variables for managing the colors
  • Changes mode based on the user's color preference
  • Added spacing between the sponsor text and the content ๐Ÿ˜ฎโ€๐Ÿ’จ

I have changed the color scheme according to my taste. Feel free to change it or let me know if any more changes are required.

@vercel
Copy link

vercel bot commented Oct 28, 2023

@MathurAditya724 is attempting to deploy a commit to the Package Phobia Team on Vercel.

A member of the Team first needs to authorize it.

pnpm-lock.yaml Outdated Show resolved Hide resolved
src/components/Sponsors.tsx Outdated Show resolved Hide resolved
@vercel
Copy link

vercel bot commented Oct 29, 2023

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Updated (UTC)
packagephobia โœ… Ready (Inspect) Visit Preview Nov 1, 2023 11:44pm

src/pages/compare.tsx Outdated Show resolved Hide resolved
@@ -12,14 +12,15 @@ const height = '4rem';
const radius = '3px';

const input: React.CSSProperties = {
color: '#333',
color: 'var(--muted-foreground)',
background: 'var(--muted)',
Copy link
Owner

@styfle styfle Oct 29, 2023

Choose a reason for hiding this comment

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

This background looks too dark in light mode

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can change it to use something like this design:

https://vercel.com/design/input#prefix-and-suffix

@styfle styfle linked an issue Oct 29, 2023 that may be closed by this pull request
@MathurAditya724
Copy link
Contributor Author

I have made all the requested changes, please have a look

@styfle
Copy link
Owner

styfle commented Oct 31, 2023

@MathurAditya724 I'm still seeing the dark gray input instead of white.

Also the search icon is hard to see.

image

https://packagephobia-git-fork-mathuraditya724-main-packagephobia.vercel.app/result?p=satori

Can you ensure that the light mode colors are not changed from the original here:

https://packagephobia.com/result?p=satori

@MathurAditya724
Copy link
Contributor Author

Some silly mistake on my end ๐Ÿ˜…, please have a look

@MathurAditya724
Copy link
Contributor Author

All good?

@styfle
Copy link
Owner

styfle commented Nov 1, 2023

@MathurAditya724 Light mode looks good now ๐Ÿ‘

image

However, dark mode has the colors flipped in the bar chart.

image

Can we keep the darker shade of green on the bottom?

@MathurAditya724
Copy link
Contributor Author

This commit should resolve this issue

src/components/BarGraph.tsx Outdated Show resolved Hide resolved
src/components/BarGraph.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Great, thanks! ๐ŸŽ‰

@styfle styfle merged commit 0169964 into styfle:main Nov 1, 2023
5 checks passed
@XhmikosR
Copy link
Contributor

@styfle just wondering, wouldn't it make more sense if the new CSS file was inlined too?

@styfle
Copy link
Owner

styfle commented Apr 16, 2024

I'm starting to think inlining is not the right solution since it can't be cached between page navigations.

@XhmikosR
Copy link
Contributor

XhmikosR commented Apr 16, 2024

That makes sense too, as long as you move all CSS to an external site.

That being said, you need cache busting along with proper cache headers, and preload if you don't want to use a blocking CSS. Your call, I just noticed the inconsistency and it's easier to just inline the newly added selectors like the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add light mode/dark mode switcher
3 participants