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

[5.x] Template color scheme configuration #42221

Merged
merged 45 commits into from Mar 11, 2024
Merged

Conversation

Fedik
Copy link
Member

@Fedik Fedik commented Oct 26, 2023

Pull Request for Issue #42134 .

Summary of Changes

Add a Color scheme switch to Atum template.
The PR allow to choose prefered color scheme based on OS setting, or fixed.
Instead of using media query (which is not possible to override) it uses data attribute (data-bs-theme for bootstrap).
Very similar to what @dgrammatiko tried in #42209 .

New API

The following data attributes should be used within template:
data-color-scheme-os When template should follow OS setting
data-color-scheme="light" Use Light scheme
data-color-scheme="dark" Use Dark scheme

The combination data-color-scheme-os data-color-scheme="dark" means that the template will follow OS setting, but initialy will be used dark color scheme.

Addittionaly the PR introduce a new JavaScript event joomla:color-scheme-change which dispatched when color scheme changes.

All of this will allow to extensions easily follow to actual template state.
And in theory, should simplify Editor fixes for specified color scheme.

SCSS fixes needed

Following SCSS code does not work:

// Inherit parent with &
.foo{
  @if $enable-dark-mode {
    @include color-mode(dark) {
      &.bar {
        background-color: var(--template-bg-dark-90);
      }
    }
  }
}
// Child element
.foo{
  @if $enable-dark-mode {
    @include color-mode(dark) {
      .bar {
        background-color: var(--template-bg-dark-90);
      }
    }
  }
}

All such instance should use:

// For fisrt example
@if $enable-dark-mode {
  @include color-mode(dark) {
    .foo.bar {
      background-color: var(--template-bg-dark-90);
    }
  }
}
// For second example
@if $enable-dark-mode {
  @include color-mode(dark) {
    .foo .bar {
      background-color: var(--template-bg-dark-90);
    }
  }
}

(Or maybe someoen know better fixes?)

After the PR will be merged someone should look on it. Or can send PR against my PR.

Testing Instructions

Apply patch run npm install.
Check the template Atum parameters Dark mode.
When its set to Follow OS settings then template color scheme should changes when OS color scheme changes.
When its set to Use Light theme or Use Dark theme then template color scheme should stay Light or Dark, always.

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: Color scheme support Manual#238
  • No documentation changes for manual.joomla.org needed

@Fedik Fedik requested a review from chmst as a code owner October 26, 2023 12:08
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Oct 26, 2023
@bembelimen
Copy link
Contributor

Hey @Fedik , looks good. Do you see also an option to add the user parameters as an extra layer, so a user can define their preferences?

@Fedik
Copy link
Member Author

Fedik commented Oct 26, 2023

Do you see also an option to add the user parameters as an extra layer

I thought about it, but I would make it in separated PR.
We need a feature that will allow to define "User specific" params in templateDetails.xml. This way it will be more flexible, than hardcoding something in to User profile form.
I think some years ago there already was a discussion about it.

@HLeithner
Copy link
Member

can't we do this in a generic way? maybe with a module and joomla w-c and event trigger and not only for atum?

Co-authored-by: Brian Teeman <brian@teeman.net>
@HLeithner
Copy link
Member

also you call it color scheme switcher, which leads to the expectation that you can more then on/off dark mode (which by the way would make sense, allowing more then just darkmode on/off) if this things is also useable for the frontend and by 3rd party

@brianteeman
Copy link
Contributor

Why not do it the bootstrap way as this is for a bootstrap template?

Specifically I am redferring to the javascript so that this can be easily toggled. Currently if i understand this correctly then its a template style setting which will mean its about 5 clicks just to toggle.

https://getbootstrap.com/docs/5.3/customize/color-modes/#javascript

@Fedik
Copy link
Member Author

Fedik commented Oct 26, 2023

can't we do this in a generic way? maybe with a module and joomla w-c and event trigger and not only for atum?

You mean for User specific? Yeah, that can work also, it will be "local setting" and in each browser user should do it again. Also will need to add a Swicth somewhere in to UI.

If you mean in general, it already is as generic as possible.

you call it color scheme switcher,

Because in Browser API it called (prefers-color-scheme: dark) 😉

Currently if i understand this correctly then its a template style setting which will mean its about 5 clicks just to toggle.

Let's be honest, how often do you need it? I will set it once and will forget it exists.

@Fedik Fedik changed the title [5.x] Color scheme switcher [5.x] Template color scheme configuration Oct 26, 2023
@brianteeman
Copy link
Contributor

Currently if i understand this correctly then its a template style setting which will mean its about 5 clicks just to toggle.

Let's be honest, how often do you need it? I will set it once and will forget it exists.

Perhaps but perhaps not

@C-Lodder
Copy link
Member

Why not do it the bootstrap way as this is for a bootstrap template?

Suppose it boils down to whether Joomla wants to simply toggle between OS, light and dark, or whether they want to offer a spectrum of themes.

I'd even go as far as saying a switcher (toggle) doesn't warrant saving the preference in the database, but instead just via local storage, seeing as it's a quick and easy action.

@brianteeman
Copy link
Contributor

I agree with you about using local storage. That is the most logical place to store an environmental setting as opposed to a site setting

@ceford
Copy link
Contributor

ceford commented Oct 27, 2023

With Firefox dark mode OFF and Dark Mode set to use Dark Theme the Toolbar button text has too little contrast. Otherwise OK.
screen shot 2023-10-27 at 08 35 01
With Firefox dark mode ON and Follow OS Settings selected the tab panels are stacked vertically and the Dark Mode list with Follow OS Settings or Use Light Theme selected has too little contrast - white on grey.
screen shot 2023-10-27 at 08 35 20
So: more work needed?


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

@Fedik
Copy link
Member Author

Fedik commented Oct 27, 2023

So: more work needed?

yes, this part is in the description under "SCSS fixes needed" title,
I have a hope it will be fixed in #42010, but not sure currently

@Septdir
Copy link
Contributor

Septdir commented Nov 1, 2023

I don't want to offend anyone, but why should the choice of color scheme be in the template settings?

This is fundamentally the wrong approach. Same as taking your browser settings to select a theme.

Does the site only have one administrator? No, there may be a lot of them. And some people use the dark theme, and some don't.
Moreover, one person may have different color schemes on different devices.

For example, when working at a computer in good lighting, I do not use dark themes. But in phone I use dark scheme, it’s very difficult for me to look at light scheme on phone.

Therefore, if you make different color schemes, then changing the scheme should be available on each page so that a person can change it at any time.

Otherwise, it is not much different from what it is now.

And once again I apologize if my words or tone offended anyone.
I just had time to look at Joomla 5 and I was simply shocked by the dark theme and the fact that I can only change it in the browser settings.

@progreccor
Copy link

I totally agree with @Septdir . It is a smart approach.

@pavluk
Copy link

pavluk commented Nov 1, 2023

I also want a color scheme switch on the admin toolbar

@fgsw
Copy link

fgsw commented Feb 6, 2024

I have tested this item ✅ successfully on 9209543


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

@Hackwar Hackwar added the PBF Pizza, Bugs and Fun label Feb 20, 2024
@adj9
Copy link

adj9 commented Feb 24, 2024

By installing the PR from scratch the administrative homa does not have the cottect style, the titles of all boxes are to be reduced.
While the other pages , such as articles and system, work
JD

@walturbo
Copy link

@adj9 idem

home

@Fedik
Copy link
Member Author

Fedik commented Feb 24, 2024

The style fix will be there:

Current PR povides only the switch feature.

@LadySolveig LadySolveig merged commit 37eaa18 into joomla:5.1-dev Mar 11, 2024
3 checks passed
@LadySolveig
Copy link
Contributor

What a very nice improvement for the dark mode - thanks @fedir for your work 🚀 🤖 and also for the support, collaboration and for testing to @bembelimen @fgsw @brianteeman

@LadySolveig LadySolveig added this to the Joomla! 5.1.0 milestone Mar 11, 2024
@Fedik Fedik deleted the dark-switch branch March 11, 2024 09:16
@Quy
Copy link
Contributor

Quy commented Mar 12, 2024

@Fedik I don't remember it being this way with PR #42986. It is probably because of this PR?

42221-dark-mode

@Fedik
Copy link
Member Author

Fedik commented Mar 12, 2024

Yes, it probably because of an issue in scss, I wrote in the fitrst comment.
Need to check that for fancy-select style.

@LadySolveig
Copy link
Contributor

LadySolveig commented Mar 12, 2024

@Fedik Created a PR, sorry that we missed your comment and messed up the Fancy-Select.
Would be happy if you could give me a test @Fedik @Quy #43008

@Quy
Copy link
Contributor

Quy commented Mar 12, 2024

When installing, it is in light mode. After installing and redirecting to login page, then it is in dark mode. Expected behavior?

@Fedik
Copy link
Member Author

Fedik commented Mar 12, 2024

After installing and redirecting to login page, then it is in dark mode.

Does your OS use Dark Mode? if yes, then it is expected I guess.

@Quy
Copy link
Contributor

Quy commented Mar 12, 2024

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dark Mode Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PBF Pizza, Bugs and Fun PR-5.1-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet