Skip to content

Simplify handling of release roadmap image class#2046

Merged
sabderemane merged 1 commit intodjango:mainfrom
adamzap:simplify-handling-of-release-roadmap-image-class
May 4, 2025
Merged

Simplify handling of release roadmap image class#2046
sabderemane merged 1 commit intodjango:mainfrom
adamzap:simplify-handling-of-release-roadmap-image-class

Conversation

@adamzap
Copy link
Copy Markdown
Member

@adamzap adamzap commented Apr 21, 2025

Due to the fact that a cookie can override the user's system setting for dark mode, the class on the release roadmap image was being managed by JavaScript.

However, we can detect all cases in CSS since the setTheme JavaScript function has updated the data-theme attribute on the body tag in all cases.

This patch also groups all CSS related to the release roadmap image together in _dark-mode.scss.


To test this, go to the download page and observe the image with all combinations of the site theme options and your system theme options.


I was assessing a refactor for switch-dark-mode.js and found this opportunity to simplify things. I wonder if we could further simplify the handling of this image, and I'll probably open an issue about that eventually.

Due to the fact that a cookie can override the user's system setting for
dark mode, the class on the release roadmap image was being managed by
JavaScript.

However, we can detect all cases in CSS since the `setTheme` JavaScript
function has updated the `data-theme` attribute on the `body` tag in all
cases.

This patch also groups all CSS related to the release roadmap image
together in `_dark-mode.scss`.

.img-release {
&.light {
html[data-theme="light"] .img-release {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These four cases should cover everything:

  1. Explicit light
  2. Explicit dark
  3. Auto light
  4. Auto dark

Copy link
Copy Markdown
Member

@sabderemane sabderemane left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you for the refactoring, definitely better like that ⭐

@sabderemane
Copy link
Copy Markdown
Member

I was assessing a refactor for switch-dark-mode.js and found this opportunity to simplify things. I wonder if we could further simplify the handling of this image, and I'll probably open an issue about that eventually.

Since I did the work I can answer, the image is generated in a way we currently can't. It was the only way to make the image not hard to read in dark theme.
If there is a way to get the image generated into both ways (dark/light), we could show the correct image of selected theme.

@sabderemane sabderemane merged commit 2eb6e0d into django:main May 4, 2025
4 checks passed
@adamzap
Copy link
Copy Markdown
Member Author

adamzap commented May 5, 2025

Ok thanks!

@adamzap adamzap deleted the simplify-handling-of-release-roadmap-image-class branch May 5, 2025 13:07
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.

2 participants