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

Enable and fix for dark mode #708

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

alphapapa
Copy link
Contributor

@alphapapa alphapapa commented Dec 7, 2023

See #707.

Copy link

netlify bot commented Dec 7, 2023

Deploy Preview for numpy-org ready!

Name Link
🔨 Latest commit 1a9fe8d
🔍 Latest deploy log https://app.netlify.com/sites/numpy-org/deploys/6604408c5c7efc0008195efc
😎 Deploy Preview https://deploy-preview-708--numpy-org.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.

@alphapapa alphapapa mentioned this pull request Dec 7, 2023
4 tasks
@alphapapa alphapapa marked this pull request as ready for review December 7, 2023 20:38
@alphapapa
Copy link
Contributor Author

@stefanv @jarrodmillman Can't request reviews on this repo, so tagging you. Thanks.

@stefanv
Copy link
Contributor

stefanv commented Dec 8, 2023

See https://deploy-preview-708--numpy-org.netlify.app/about/; what do we do with those logos?

@alphapapa alphapapa marked this pull request as draft December 8, 2023 00:34
@alphapapa
Copy link
Contributor Author

alphapapa commented Dec 8, 2023

See https://deploy-preview-708--numpy-org.netlify.app/about/; what do we do with those logos?

Apologies, I should have checked the other pages as well.

I think those logos can be decently handled by filters, like the smaller ones on the tab sections. I'll see what I can do.

Marking as a draft until ready for review again. Thanks.

@jarrodmillman
Copy link
Member

See #712.

@alphapapa
Copy link
Contributor Author

Looking at the logos again, as Stefan asked.

Two options come to mind:

  1. For ones with poor contrast, we could use a CSS filter to invert the colors. That would likely fix the contrast, although it would, well, invert the colors. :)
  2. We could also adjust the background color, or try using a drop-shadow filter, something like that, to gently improve contrast without adjusting the image itself.

By the way, I noticed that the sponsors.html shortcode includes this stylesheet, which isn't functional because it's not in the page header:

<style>
.sponsor-images {
display: flex;
flex-direction: row;
max-width: 900px;
justify-content: center;
margin: 30px;
}
.sponsor-images > a {
height: 75px;
width: auto;
padding: 0 30px;
margin: 30px 0;
}
@media only screen and (max-width: 850px) {
.sponsor-images {
flex-direction: column;
}
.sponsor-images > img {
height: auto;
margin: 30px 0;
padding: 0;
}
}
</style>

The partners.html file includes one as well:

<style>
.partner-images {
display: flex;
flex-direction: row;
max-width: 900px;
justify-content: center;
margin: 30px;
}
.partner-images > a {
height: 75px;
width: auto;
padding: 0 30px;
margin: 30px 0;
}
@media only screen and (max-width: 850px) {
.partner-images {
flex-direction: column;
}
.partner-images > img {
height: auto;
margin: 30px 0;
padding: 0;
}
}
</style>

We should probably replace those with either inline styles on the relevant elements, or generalize them into the page's stylesheet.

@stefanv @jarrodmillman

Please let me know what you'd prefer that I do about them. Thanks.

@stefanv
Copy link
Contributor

stefanv commented Feb 9, 2024

Sometimes, images can be used in light and dark mode, but we definitely need the ability to specify different images for each. How about something like this:

<img src="img-dark.png" class="dark-theme"/>
<img src="img-light.png" class="light-theme"/>

Then, in the stylesheet:

html[data-theme="dark"] {
  .light-theme {
    display: none;
  }
}

html[data-theme="light"] {
  .dark-theme {
    display: none;
  }
}

@stefanv
Copy link
Contributor

stefanv commented Feb 9, 2024

Re: sponsors.html, it should get its own .css file; or we remove sponsors.html and att it to numpy.org, since I think that's the only place it is being used.

@alphapapa
Copy link
Contributor Author

alphapapa commented Feb 9, 2024

Re: sponsors.html, it should get its own .css file; or we remove sponsors.html and att it to numpy.org, since I think that's the only place it is being used.

@stefanv Sorry, I'm not sure what you mean about moving sponsors.html to numpy.org.

@alphapapa
Copy link
Contributor Author

alphapapa commented Feb 9, 2024

Sometimes, images can be used in light and dark mode, but we definitely need the ability to specify different images for each. How about something like this:

@Stefan Sure. I think we could also use the picture element to do the same thing without having to add CSS classes. What do you think?

alphapapa added a commit to alphapapa/numpy.org that referenced this pull request Feb 9, 2024
@alphapapa
Copy link
Contributor Author

Pushed #725 in re #708 (comment)

@jarrodmillman
Copy link
Member

jarrodmillman commented Feb 9, 2024

The style stuff in the shortcodes appears to be functional to me. It appears in the source file and when you remove the styling from the shortcode the page looks different. For example, with the styling
2024-02-09T14:17:22,330802550-08:00
and without it
2024-02-09T14:18:46,025201639-08:00

So we can probably just leave it as is for now. Or move it.

@alphapapa
Copy link
Contributor Author

The style stuff in the shortcodes appears to be functional to me. It appears in the source file and when you remove the styling from the shortcode the page looks different. For example, with the styling 2024-02-09T14:17:22,330802550-08:00 and without it 2024-02-09T14:18:46,025201639-08:00

So we can probably just leave it as is for now. Or move it.

Hm, ok, according to this, it's not officially supported yet, but unofficially it works.

I'll close #725 then. Thanks.

@stefanv
Copy link
Contributor

stefanv commented Feb 9, 2024 via email

@alphapapa
Copy link
Contributor Author

I considered that, but we do not merely rely on them preference, we also allow setting it, and that appears as a data element on html.

Thanks.

@jarrodmillman
Copy link
Member

Once we get the grid sorted out, maybe these can just be grids of images (i.e., maybe we don't need special roles for sponsors and partners).

@jarrodmillman jarrodmillman changed the title Enabling and fixing for dark mode Fixes for dark mode Mar 27, 2024
@jarrodmillman jarrodmillman changed the title Fixes for dark mode Enabling and fixing for dark mode Mar 27, 2024
@jarrodmillman jarrodmillman changed the title Enabling and fixing for dark mode Enable and fix for dark mode Mar 27, 2024
This is a simple way to make them "presentable" in dark mode.
Otherwise, we'd need to keep their backgrounds white (which would
clash with the dark background), or edit each one individually and
swap them for dark mode, which would be more complicated.
This seems to look good, and it avoids the clash of having some bright
white, light-mode images when using dark-mode.  Since all of these
images are illustrations and diagrams, not photographs, it seems good.
Before this change, the images in the div.grid-container were
displayed about one line higher than the text in the adjacent
paragraph.
@jarrodmillman
Copy link
Member

Let's put this on hold for a bit b/c I want to revisit if we can use theme functionality directly.

@jarrodmillman
Copy link
Member

jarrodmillman commented May 5, 2024

See scientific-python/scientific-python-hugo-theme#599 and #746. Still working on things, but there is progress.

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