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

Feature suggestion: use mw.user.clientPrefs library and consistent classes with evolving skins #780

Open
jdlrobson opened this issue Feb 2, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@jdlrobson
Copy link
Contributor

The Problem

You don't need to do anything here, but you might benefit from the upstream changes that are occurring!

We've started implementing night mode in Vector 2022 and Minerva and one thing we're trying to do as part of that is standardize classes across skins for how night mode should behave.

You may want to use the same class names on the HTML element and the same API (mw.user.clientPrefs) for managing the mode for anonymous users (it works very similar to your existing window.applyPref)

Let me know if you have any questions or curiosities or lessons learned from implementing night mode in Citizen!

You can check out the following patches to see how night mode is evolving in the Minerva skin:

The Solution

The classes we've landed on are skin-night-mode-clientpref-0 (night mode disabled) = skin-night-mode-clientpref-1 (night mode enabled), skin-night-mode-clientpref-2 (automatic night mode)

The benefit of using this class is that you can make night mode persist for anonymous users.
e.g. calling `mw.user.clientPrefs.set( 'skin-night-mode', '1' ); should enable night mode and persist it for anonymous users.

@jdlrobson jdlrobson added the enhancement New feature or request label Feb 2, 2024
@alistair3149
Copy link
Member

Thank you for the detailed message!
I have been tracking the development of skin architecture and it is awesome to see the progress being made various fronts, especially with dark mode!

As for mw.user.clientPrefs, it is very useful to have an official API for both anon preferences and avoiding FoUC.

Because Citizen has to maintain support for the latest MW LTS (1.39), this will be implemented on the next MW LTS stable release, along with a proper implementation of Codex design tokens. However, I am able to add the night mode classes right away if that is already standardized and ready for adoption.

Let me know if you have any questions or curiosities or lessons learned from implementing night mode in Citizen!

The most important lesson that I learned was that: Because skins can't control most of the UI, skins need provide means to create a cohesive UI.

I'm sure that you realized too because it is a recurring theme on WMF Phabricator for dark mode and DIP. Skins are only responsible for the outer frame of a page.

  • Extension and MW core styles: Citizen has specific skinStyles for lots of extensions and MW core RL modules that but it is not sustainable. Thankfully, Codex is here to solve that but it still has a long road ahead, especially for third party extensions.
  • Content styles: Citizen uses CSS variables so wikis can utilize those CSS variables in site and template styles, ensuring consistency in their design. I do think that MediaWiki should have a way to expose the final Codex tokens to end users, that will help wikis create a more unified design, while allowing skins to have more control over the general styles of the wiki.

@jdlrobson
Copy link
Contributor Author

Regarding content styles - yes very similar approach is planned for Wikimedia with the caveat that currently TemplateStyles does not support CSS variables (https://phabricator.wikimedia.org/T320322)

For extensions/MW core styles we plan to make use of Codex design tokens (we are currently exploring making them accessible as CSS variables in https://phabricator.wikimedia.org/T356537)

A sneak peek of our incremental progress can be seen here: https://m.mediawiki.org/?minervanightmode=1

alistair3149 added a commit that referenced this issue Apr 24, 2024
…ntPrefs

This is the first step of migrating to the clientPrefs library when it is avaliable.
Currently only themes are using it and the standard classes are added to the HTML element.

Related: #780
alistair3149 added a commit that referenced this issue Apr 25, 2024
Since clientPrefs and other related features are not avaliable until MW 1.42,
many of them are backported as polyfill. Instead of using cookies, the polyfill
are using localStorage only like Citizen in the past.

There are many changes behind the scene, but the most important one being that
`skin-citizen-*` theme classes are now soft-deprecated, and replaced by the
standardized `skin-theme-clientpref-*` classes. There will be sufficient
time before the hard deprecation.

Related: #780
@alistair3149 alistair3149 pinned this issue Apr 25, 2024
@alistair3149
Copy link
Member

Since mw.user.clientPrefs is only available since MW 1.41, we have polyfilled a version that uses localStorage. It should allow for an easy change-over once we are ready to drop MW 1.39 support and move to MW 1.43.

@jdlrobson
Copy link
Contributor Author

Also heads up after feedback the class has changed to skin-theme-clientpref-night and skin-theme-clientpref-os for night mode in Vector and Minerva. We realized we were making a theming system and were asked to have human readable classes instead of 0,1 and 2.

@alistair3149
Copy link
Member

Also heads up after feedback the class has changed to skin-theme-clientpref-night and skin-theme-clientpref-os for night mode in Vector and Minerva. We realized we were making a theming system and were asked to have human readable classes instead of 0,1 and 2.

Thanks for the heads-up! I have read the relevant patches and the new classes are the version that we implemented.

On a side note, would the i18n messages for the theme (skin-theme-) be merged into MW core, given the message key prefix, and are shared across Vector and Minerva? When we implement the same key, it caused an issue with TranslateWiki where they don't allow the same message key across different repos.

@jdlrobson
Copy link
Contributor Author

Sorry for the delayed reply. There are no plans right now to move these messages to core as they are tied to the Vector and Minerva skins. If there was a way we could abstract things that made sense we could explore that.

Are you planning to load the same controls in Citizen or will you continue to use your own interface? If the latter it is probably best to prefix - i believe translatewiki allows you to sync your messages with another message key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants