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

Add dark theme #102

Merged
merged 15 commits into from
Sep 24, 2023
Merged

Add dark theme #102

merged 15 commits into from
Sep 24, 2023

Conversation

bmartinez287
Copy link
Collaborator

@bmartinez287 bmartinez287 commented Sep 23, 2023

The Issue

The ddev website does not currently have a dark theme.
The current theme is beautiful but it does hurt my eyes a bit under low light conditions or late at night.

Before

Screen Shot 2023-09-23 at 11 17 05 AM

After

Screen Shot 2023-09-23 at 11 16 53 AM

How This PR Solves The Issue

This PR adds the tailwindcss dark theme classes to the current components.

Manual Testing Instructions

To test it, set your browser to use a dark mode and then visit the website.
This uses a browser's prefers-color-scheme: dark setting.

@rfay rfay marked this pull request as draft September 23, 2023 19:49
@rfay
Copy link
Member

rfay commented Sep 23, 2023

Thanks for this! I moved it to "draft" until you're ready for Matt to have a look at it. Then you can just change to "Ready for review"

@bmartinez287
Copy link
Collaborator Author

It's ready for review. I checked all the blog posts and the quickstart guide to make sure they look good in both dark and light mode. Let me know if something needs to be tweaked.

@bmartinez287 bmartinez287 marked this pull request as ready for review September 23, 2023 21:41
@rfay rfay requested a review from mattstein September 23, 2023 21:46
Copy link
Collaborator

@mattstein mattstein left a comment

Choose a reason for hiding this comment

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

Wow, thanks for taking this on @bmartinez287! 👏

It looks good—played with it locally and leaving suggestions for maintaining a similar visual balance in dark mode:

  • Careful with logos, avatars, and brand colors!
  • Anywhere you’ve got prose, you can use dark:prose-invert for consistency and a canned set of considerations for that text.
  • Keep borders a little soft.
  • Include a few missing odds and ends.

Comments on unchanged files:

  1. src/components/Badge.astro should get a softer dark border with dark:border-slate-700.

  2. Those big CTA buttons should probably get brighter on hover; in src/components/CtaButton.astro, try adding dark:hover:text-blue-500 hover:border-blue-500 for the hollow classes on line 11.

  3. You could soften the footer’s border by adding dark:border-slate-700 to line 116 in src/components/Footer.astro.

  4. You could soften the “Get Started” platform tab borders by adding dark:border-slate-700 to each of the label elements. (Ones with for="platform-*" attributes.)

  5. You could soften the repo card’s borders (src/components/RepoCard.astro) by adding dark:border-slate-700 to the wrapping anchor and the div element immediately wrapping the h3.
    Screen Shot 2023-09-23 at 08 57 59 PM@2x

  6. The “Get Started” tabs’ active state and balance is a little funky (src/components/Tabs.astro). I think the following works better, but see if you agree:
    a. Remove the classes modifying these elements from src/styles/global.css.
    b. Add dark:text-gray-300 to the &.active { block (line 49).
    c. Add dark:text-gray-300 to the span that follows (line 52).
    d. Add dark:bg-transparent dark:shadow-slate-900/50 to the .panel { block (line 58).
    e. Remove the class that was added to the span element on line ~122.
    Screen Shot 2023-09-23 at 08 59 43 PM@2x

  7. On the “Newsletter” page (src/pages/newsletter.astro), wrap the one paragraph in <div class="prose dark:prose-invert my-8">. (I cheated and added some vertical breathing room there, too!)

  8. On the “Search” page (src/pages/search.astro), make sure the search input and results are styled intentionally.
    a. Add dark:border-slate-700 to the result anchor (line 49).
    b. Add dark:text-white to the h2 element.
    c. Add dark:text-slate-500 to the div element already having inline-block text-sm font-mono classes.
    d. Add dark:bg-transparent dark:border-slate-700 dark:text-white to the #search-keywords input.

@@ -20,7 +20,7 @@ const logos = [
logos.map((logo) => (
<div class="px-12 py-6">
<img
class={["h-12", ...logo.extraClasses].join(" ")}
class={["h-12 dark:invert", ...logo.extraClasses].join(" ")}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a tough one because this changes colors that definitely won’t conform to several organizations’ brand guidelines. (It’s one thing that kept me from trying to tackle dark mode!)

I’m not sure how to handle that unless it’s on a case-by-case basis:

  • Platform.sh looks great inverted, and that’s probably acceptable brand-wise.
  • Tag could probably invert the letterforms and leave the bracket colors.
  • 1x Internet may be able to stay red, or go all white.
  • i-gelb could leave the yellow dot and invert the text and other strokes.
  • Agaric may be able to work as-is, or just go all white.
  • b13 looks great inverted to white, and I would guess that’s fine.
  • Gizra would probably work in its original color.
  • Oliver Wand could skip inversion and look normal / not demonic.
  • Centarro could also skip inversion and work fine.
  • DrupalEasy looks good inverted, so that’s probably fine.
  • Redfin Solutions’ text should probably go white, with the logo mark’s color in its original form.
  • MacStadium’s treatment could work like Redfin Solutions’.
  • Lullabot’s text could probably be inverted with the logo mark in that red—assuming there’s enough contrast that it reads okay.
  • Craft CMS can exist in its original form and it’d work great.
  • undpaul could have only its text inverted to white and that’d probably work well.

This is a pain to solve, so it may be worth separating into a separate issue.

It might be simplest to create additional dark logo assets for each sponsor, and fill GitHub Sponsor bubbles with white since they have a similar issue:

Screen Shot 2023-09-23 at 09 11 51 PM@2x

If we have dark-mode-appropriate logos, we might also be able to solve the same problem in the ddev/ddev readme.

I realize it’s not the end of the world, I just try to be careful with the images people and organizations choose to represent themselves.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the white background to those logos and the sponsor's logo I worked with each one of them. If the invert would not work I gave them a white background.

src/components/Benefits.astro Outdated Show resolved Hide resolved
src/components/BlogPostFooter.astro Outdated Show resolved Hide resolved
src/components/Footer.astro Outdated Show resolved Hide resolved
src/components/quickstart/CommunityCTA.astro Outdated Show resolved Hide resolved
src/pages/support-ddev.astro Outdated Show resolved Hide resolved
src/pages/support-ddev.astro Outdated Show resolved Hide resolved
@@ -20,6 +20,43 @@ footer {
}
}

@media (prefers-color-scheme: dark){

header {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The DDEV logo’s text works great in white, but the current inverted orange should stay blue. This is easier than sponsor logos because the SVG itself could be embedded in the page with IDs used in selectors that change the text color.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That sounds good though the orange would make it easier for others trying to have a dark theme with a DDev logo. Any chance we could adopt the orange as a dark equivalent to the blue? I kind of like the orange as an easier alternative and visual indication we are in the dark theme.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The entire layout being inverted seems strong enough an indication to me that dark mode is active and I wouldn’t recommend altering core brand colors to suit dark mode—but I’d leave that up to @rfay.

The green external link icons are a unique departure too, but I don’t have any objection there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's true for ddev.com but on pages where only the logo represents the brand. it could be the dark version of our logo so we don't have the same issue some of the other logos had.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we’re describing the same thing! (And most of those sponsor logos could have alternate versions in the same way—need to create the variations but I could either add them here or in a separate PR.)

This is what I’m trying to describe doing with the DDEV logo that doesn’t introduce new brand colors:

ddev-logo-reversed-example

This is an optimized SVG for exactly that (no background) if you want to use it:

ddev-logo-reversed

Could swap the whole thing or be more efficient and select only the text nodes within the SVG.

src/styles/global.css Outdated Show resolved Hide resolved
src/styles/global.css Outdated Show resolved Hide resolved
@bmartinez287
Copy link
Collaborator Author

bmartinez287 commented Sep 24, 2023

I think I got them all. I went down the list one by one. Let me know when you get a chance. Thanks for such a comprehensive review. It definitely helped speed this along.

@mattstein
Copy link
Collaborator

I think I got them all. I went down the list one by one. Let me know when you get a chance. Thanks for such a comprehensive review. It definitely helped speed this along.

Thanks for doing that, and sorry it was such a long list. I probably should have made a PR on your PR or something. I’ll have a look again today.

@bmartinez287
Copy link
Collaborator Author

No worries, it was great having so many items to reference all at once.

@bmartinez287
Copy link
Collaborator Author

Screen Shot 2023-09-24 at 12 17 20 PM I added the SVG

@bmartinez287
Copy link
Collaborator Author

bmartinez287 commented Sep 24, 2023

So that just leaves two minor details. Hopefully.

In the image below there are a few items that could be improved by getting a dark version of their logo.
Tag1 - Lullabot and I-gelp look ok as they are but they could look better.

Screen Shot 2023-09-24 at 12 19 59 PM

By adding the white background to the bubbles below that issue seems fixed to me.
Screen Shot 2023-09-24 at 12 20 04 PM

@mattstein
Copy link
Collaborator

I can follow up with a PR to improve the Featured Sponsors logo handling. The contributor bubbles are a solid win though!

Looked again locally and it seems like a few things are missing unless you decided against them or my suggestions were off:

  1. The “Getting Started” tab panels still use a black underline for the active state, include an awkward background shade, and either remove the drop shadow or use an invisible one:
    Screen Shot 2023-09-24 at 11 31 59 AM@2x
  2. The blog post paging’s back/previous anchor still has a loud border:
    Screen Shot 2023-09-24 at 11 32 47 AM@2x
  3. The about page sidebar stats are black:
    Screen Shot 2023-09-24 at 11 33 41 AM@2x

Otherwise everything looks good to me!

@bmartinez287
Copy link
Collaborator Author

bmartinez287 commented Sep 24, 2023

Screen Shot 2023-09-24 at 12 47 31 PM

I added a dark version of the SVGs that were hard to read.

@mattstein
Copy link
Collaborator

I added a dark version of the SVGs that were hard to read.

That’s way better, and I like the approach you took! 👏

I’d probably still make some fine adjustments, but you’ve done most of the work and I’m happy to pick at those myself separately.

@bmartinez287
Copy link
Collaborator Author

It should be ready for review. I got those last fixes in. Let me know if there's anything else.

Copy link
Collaborator

@mattstein mattstein left a comment

Choose a reason for hiding this comment

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

Looks great to me now! Thanks again for tackling this and being quick and diligent with my comments @bmartinez287. 🙌

@mattstein
Copy link
Collaborator

@rfay Do you have anything to add or should we merge this?

@rfay
Copy link
Member

rfay commented Sep 24, 2023

Nothing to add! Thanks so much for this @bmartinez287 and thanks so much for caring for it so thoroughly @mattstein !

@mattstein mattstein merged commit 1adca41 into ddev:main Sep 24, 2023
1 check passed
@rfay
Copy link
Member

rfay commented Sep 24, 2023

🚀

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