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

[WIP][5.0] Dark Mode support #41409

Merged
merged 21 commits into from
Sep 3, 2023
Merged

Conversation

wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Aug 20, 2023

Please note this is still miles away from even being ready for testing - but is just here so that the work is being done transparently and is able to be tracked for maintainers & others.

Summary of Changes

Enables dark mode support for Joomla 5 on the back of the Bootstrap 5.3 upgrade of J4.

Testing Instructions

This requires compiling of CSS so won't work with regular patchtester

Use dark mode on your browser (usually exposed through an OS Setting) and browse around the backend validating that visually dark mode is enabled and consistent.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Aug 20, 2023
@Quy Quy mentioned this pull request Aug 21, 2023
@HLeithner
Copy link
Member

thanks for starting this pr, I'm adding a screenshot for now
image

@wilsonge
Copy link
Contributor Author

Please note this is still miles away from even being ready for testing

:P

@wilsonge wilsonge force-pushed the feature/dark-mode branch 2 times, most recently from 31118b4 to 1f166f9 Compare August 25, 2023 22:27
@web-eau-net
Copy link

I have tested this item 🔴 unsuccessfully on 4069ac1

Dark mode activated in Opera 101.0.4843.43 but does not affect the backend :(


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41409.

@rabidgrowth
Copy link

(I am writing to you with Google Translate because I can read English but not write, as you may have seen)

Success!

Windows 11 with darkmode enabled, Google Chrome fully updated, PHP 8.2.

He still wants a job, as winsonge said. Still, congratulations, it's well on its way! We needed darkmode in Joomla, especially us who work nights LOL!

For the friends who reported failure, it doesn't work with patchtester. I think the patchtester doesn't work if the media has been changed. Maybe put a note so people don't get confused? Anyway, I did a git clone and ran composer install and npm ci as it says in the README.

Install and admin play fine with darkmode!

Colors are generally okay, but some are not very good for darkmode. I'll tell you what I saw with a quarter of an hour of testing it:

  • The quickicons have a very bright background. So are all the buttons on the toolbar.
  • I went to make a new article. The background and foreground on the tabs are very bright. I can't read the tabs.
  • TinyMCE is very bright, even if I change his skin. I guess like in joomla 4 it needs editor.css in the template?
  • One of the same and CodeMirror. In Joomla 4 I could change his theme and make him play with Dracula. This option is gone. Will it come back?
  • Going to make menuitem the Menu Item Type and Link have very bright background and foreground. I can't read what they are saying. From what I can see this happens anywhere you have a button that loads a popup or a disabled control.
  • Where I have to put image the background is too bright until I select photo / image.
  • In the multifactor authentication of the user, the colors are completely different for elsewhere. I can't read anything.
  • In the extensions installation the background where I dragadrop the files is basically white and pops out.
  • None of the help is darkmode, but I see it loads from another site, so it's probably not related to this pr.

I hope I've helped you and I'm sorry to bother you these days!

@wilsonge
Copy link
Contributor Author

Thankyou that is actually very helpful!! I've fixed tabs in article edit and the quickicons.

I will work my way through the rest of your list. Thankyou. thankyou. thankyou!

@wilsonge
Copy link
Contributor Author

So not saying I'm done by any means (but it's definitely slowly getting there).

I've reached the point where we need to think about how extensions interact with this without necessarily becoming hard bootstrap coupled. So as an example we can look at media manager integration (component), or extension drag and drop (plugin) and harder still editor theme changing (tinymce/codemirror - harder because we need to decide if a custom theme even supports dark mode - we know the default ones do - as @dgrammatiko showed in his PR)

I'd appreciate some thoughts on this from @dgrammatiko , @HLeithner, @laoneo or anyone else for that matter

@dgrammatiko
Copy link
Contributor

I've reached the point where we need to think about how extensions interact with this without necessarily becoming hard bootstrap coupled.

That's a great call for design tokens. A design system backing the tokens would also be nice

harder because we need to decide if a custom theme even supports dark mode

It's not that hard. The moving parts here are:

  • DocumentElement attribute that denotes if a theme is forced (custom ones other than light/dark are always forced!)
  • DocumentElement attribute that has the name of the theme (light/dark/dimitris) that would be picked from the CSS/JS
  • Server side rendered parts just follow the design token (basically Joomla named CSS vars)
  • Client side apps (tinyMCE/etc) just have to have the logic for forced/OS options (check the Muta implementation)

@wilsonge
Copy link
Contributor Author

I’m not using themes directly in this version. Just media queries. But that’s going to have to change for extension integrations with the template. Can just be an options setting I guess - but doubt CSP will like toggling css files based on the media query. So likely somehow declared in PHP??

@rabidgrowth
Copy link

(I am writing to you with Google Translate)

I thank you for all the work you did @wilsonge !

Regarding the new expansions, you put a lot of thought into it. See what Bootstrap says here: https://getbootstrap.com/docs/5.3/customize/color-modes/#sass-mixins

In my templates with Bootstrap 5.3 I do something like the following:

@import 'sections/home';

@include color-mode(dark) {
@import 'sections/home_dark';
}

In _home.scss I have the custom CSS I want for lightmode. This has everything in it, flexbox, margins, padding, and natural colors. In _home_dark.scss I only have colors. Bootstrap's color-mode automatically plays with $color-mode-type. If I change my mind, I recompile my SCSS. Simple and quick, without much fuss.

Don't overcomplicate it. Most sites are made by small companies with 2-3 people. Unnecessary complexity kills our profit. The customer pays by the piece, not by the hour.

@HLeithner HLeithner marked this pull request as ready for review September 3, 2023 12:54
@HLeithner
Copy link
Member

After CI fix I merge this for beta1, we know additional things are needed but for now it's a good start.

@wilsonge
Copy link
Contributor Author

wilsonge commented Sep 3, 2023

CI Fixed

@HLeithner HLeithner merged commit d157efc into joomla:5.0-dev Sep 3, 2023
1 of 3 checks passed
@HLeithner
Copy link
Member

Thanks, docs needed

@wilsonge wilsonge deleted the feature/dark-mode branch September 3, 2023 14:23
@brianteeman

This comment was marked as abuse.

@Quy
Copy link
Contributor

Quy commented Sep 3, 2023

41409-config
41409-dashboard
41409-inline-help
41409-hover
41409-permissions
calendar

@Quy
Copy link
Contributor

Quy commented Sep 13, 2023

41409-media

edit-style

mfa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants