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: dark/light mode theme regression #1862

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

bdkopen
Copy link

@bdkopen bdkopen commented Apr 5, 2025

This fixes a minor dark/light mode theme regression introduced in #1839.

The display type was switched from block to flex, but the CSS wasn't updated for other uses across the site. For example, the social icons on the website footer became uncentered because some icons don't receive the dark/light mode classes. As another example, the logos on the community page lost a little bit of height as flex.
Screenshot 2025-04-05 at 12 11 59 AM
Screenshot 2025-04-05 at 12 22 16 AM
Screenshot 2025-04-05 at 12 26 54 AM

The simplest solution is to give the logo introduced in #1839 it's own dark/light mode class setting display: flex class and let the rest of the site use display: block.
Screenshot 2025-04-05 at 12 12 09 AM
Screenshot 2025-04-05 at 12 23 13 AM
Screenshot 2025-04-05 at 12 26 29 AM

@bdkopen bdkopen requested review from a team as code owners April 5, 2025 04:28
Copy link

netlify bot commented Apr 5, 2025

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 19198c3
🔍 Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/67f0b163a91d48000831f97a
😎 Deploy Preview https://deploy-preview-1862--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<img src="/images/brand/logotype-light.svg" alt="Express.js logo" width="90" height="30"/>
</a>
<a href="/" class="hidden-light" title="Go to homepage" aria-label="Go to homepage">
<a href="/" class="hidden-light-flex" title="Go to homepage" aria-label="Go to homepage">
Copy link
Member

Choose a reason for hiding this comment

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

we should use one svg icon and change color as theme changes

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, although I don't remember why I didn't do it that way from the start. There must have been a reason, but I think we can try it again.

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