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

Reinstate dark mode framework (with dark mode disabled) #1954

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

holly-cummins
Copy link
Contributor

#1951 reverted #1908, to resolve #1950 (which affected light mode), and also to solve some issues with dark mode.

  • search icons are not showing
  • in some guides, images with a transparent background do not render well

However, it's desirable to have the #1908 changes in the code base. One reason is that we want to fix these minor issues and have dark mode, because people ask for it. Doing these fixes will be easier, and lower risk, if there's not a major merge to bring back #1908. Another reason is that #1908 removes some hardcoded colours and introduces more semantically-meaningful variable names, which is a good thing even if we never do dark mode.

This PR reintroduces #1908, but with the code background colour corrected, and with dark mode always-off.

image

We should do a thorough visual review of the preview to make sure there are no other light mode regressions (doing this kind of colour-to-variable refactoring can be error-prone, which is why we want to only do it once!)

cc @insectengine

@holly-cummins holly-cummins force-pushed the reinstate-dark-mode-framework branch from 24fa221 to 78c902e Compare April 18, 2024 17:21
Copy link

🎊 PR Preview d790114 has been successfully built and deployed to https://quarkus-site-pr-1954-preview.surge.sh

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

Copy link

😭 Deploy PR Preview failed.

@holly-cummins
Copy link
Contributor Author

Next steps after this merges:

  • Add a toggle for dark mode (possibly hidden, at first)
  • Fix search icon rendering in dark mode
  • Adjust 'extensible' content such as guides and blogs to put in a light-coloured background on all images. This ensures all images are legible (we could eventually also introduce a naming convention to allow an override for images which are declared to work in both modes and want a transparent background)
  • Carefully review dark mode for other visual problems

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

+1 code blocks is readable and dark mode is disabled so can roll out more safely

@holly-cummins
Copy link
Contributor Author

Oh, and the other next step is to replicate the toggle mode to http://quarkus.io/extensions, which is at the moment using system settings, and should be switched to light mode (while waiting for the toggle)

@maxandersen
Copy link
Member

maxandersen commented Apr 18, 2024

examples of pages having "bad" darkmode:

http://0.0.0.0:4000/blog/quarkus-kiota/ - code block with darkgrey on grey
http://0.0.0.0:4000/blog/monitoring-quarkus-jvm-mode-with-cryostat/

2024-04-18_20-08-49

http://0.0.0.0:4000/blog/quarkus-saas-backend-time-tracking-app-case-study/ - transparent logo unreadable

2024-04-18_20-06-47

@gsmet
Copy link
Member

gsmet commented Apr 19, 2024

@maxandersen your comment above is not about this PR, right, it's when you enabled dev mode yourself? We can merge it?

@holly-cummins
Copy link
Contributor Author

@maxandersen your comment above is not about this PR, right, it's when you enabled dev mode yourself? We can merge it?

Answering for @maxandersen, we don't have a good place to record the task list for dark mode, so he's put it on this work item for now. All of the problems he mentioned are technically in this changeset, but could never be seen by a user. But I will let @maxandersen also answer for himself!

@maxandersen
Copy link
Member

Correct - it was just when i enabled dark mode. I added it here as way to collect the info.

For the actual PR I'm all +1 - code reads, looks and behaves fine.

@gsmet gsmet merged commit 07aa4b3 into quarkusio:develop Apr 22, 2024
1 check passed
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.

Illegible colour scheme in the code snippets
3 participants