MediaWiki talk:Gadget-dark-mode-toggle.js

No reload required

@SD0001: You can just do something like this.

if (inDarkMode) {
	var css = mw.loader.moduleRegistry['ext.gadget.dark-mode'].style.css[0];
	$('style').filter(function () {
		return this.textContent.includes(css);
	}).prop('disabled', inDarkMode);
} else {
	mw.loader.load('ext.gadget.dark-mode');
}

Nardog (talk) 03:11, 27 November 2021 (UTC)

@Nardog With dark mode on, mw.loader.moduleRegistry['ext.gadget.dark-mode'].style is undefined for me? – SD0001 (talk) 03:19, 27 November 2021 (UTC)

It seems the style attribute shows up in the module registry only if the gadget is loaded using mw.loader.load('ext.gadget.dark-mode');. If it was already loaded at page load (using user option), there's no style attribute in the registry even if mw.loader.load('ext.gadget.dark-mode'); is explicitly run.

Anyway, nice suggestion! I suppose we can still load the styles via an AJAX call to bypass the reload requirement (though there seems to be an issue here as this css is somewhat different in terms of whitespace). Thanks. – SD0001 (talk) 03:35, 27 November 2021 (UTC)

(edit conflict) Oh yeah, you're right, the CSS is shipped with other modules in <link> when turned on as a gadget so it won't work... my bad. It can still be achieved by, say, fetching the CSS remotely and iterating through document.styleSheets and deleting the matching rules, or fetching the same modules except the dark mode, disabling the original stylesheet, and adding the other modules again, but that's obviously too much kludge. But turning it on (and turning it off after it was turned on via this script) can still be done without reloading.

(And I should have added mw.user.options.set('gadget-dark-mode', inDarkMode ? 0 : 1); to make toggling more than once possible.) Nardog (talk) 03:44, 27 November 2021 (UTC)

I found a much simpler solution: You can just remove dark-mode from <link href="...">. Implementing it isn't so simple if you want it to be reliable, though, unfortunately. This was all I could come up with but there may be a simpler solution with just regex replacements:

// declare mediawiki.Uri

var css = mw.loader.moduleRegistry['ext.gadget.dark-mode'].style;
if (css) {
	css = css.css[0];
	$('style').filter(function () {
		return this.textContent.includes(css);
	}).prop('disabled', inDarkMode);
} else if (inDarkMode) {
	$('link[rel="stylesheet"][href^="/w/load.php?"]').attr('href', function () {
		var uri = new mw.Uri(this.href),
			modules = uri.query.modules;
		if (!modules) return;
		modules = modules.split('|');
		var gadgetsIdx = modules.findIndex(function (s) {
			return /^ext\.gadget\..*dark-mode/.test(s);
		});
		if (gadgetsIdx === -1) return;
		var gadgets = modules[gadgetsIdx].slice(11).split(',');
		gadgets = gadgets.filter(function (s) {
			return s !== 'dark-mode';
		});
		if (gadgets.length) {
			modules[gadgetsIdx] = 'ext.gadget.' + gadgets.join(',');
		} else {
			modules.splice(gadgetsIdx, 1);
		}
		if (modules.length) {
			uri.query.modules = modules.join('|');
		} else {
			delete uri.query.modules;
		}
		return uri.getRelativePath();
	});
} else {
	mw.loader.load('ext.gadget.dark-mode');
}

Nardog (talk) 05:14, 27 November 2021 (UTC)

@Nardog Yes using regex seems much simpler. Also once we've manipulated the <link> element, the mw.loader.load no longer works if the user wants to turn on dark mode again, unless we also manipulate mw.loader.moduleRegistry['ext.gadget.dark-mode'].state after the link tag manipulation. – SD0001 (talk) 12:13, 27 November 2021 (UTC)

At this point, I realised that it would be better to avoid both sets of hacks – link tag manipulations + use of mw.loader features which I hardly think are considered parts of the stable interface, and use just the former for all cases. See the latest version. I've listed you as co-author. Let me know if you find any issues. – SD0001 (talk) 12:19, 27 November 2021 (UTC)

mw.loader.state() is the interface for it, but your solution works. Nardog (talk) 07:35, 28 November 2021 (UTC)

Automatically toggle based on OS dark mode setting

Windows and macOS both have dark mode now. It should be possible to provide an opt-in feature for automatically toggling dark-mode when the OS preferred colour scheme changes. – SD0001 (talk) 13:58, 28 November 2021 (UTC)

Done in Special:Diff/1057626461 – can be enabled by putting window.wpDarkModeAutoToggle = true in the common.js page. – SD0001 (talk) 19:01, 28 November 2021 (UTC)

Interface-protected edit request on 16 December 2021

Change

	var portlet = mw.config.get('skin') === 'minerva' ? 'pt-preferences' : 'p-cactions';
	mw.util.addPortletLink(portlet, '#', label, 'pt-darkmode', tooltip);

to

	mw.util.addPortletLink('p-personal', '#', label, 'pt-darkmode', tooltip);

as suggested by MusikAnimal and Enterprisey at VPT.

Also, currently the script puts text where an icon is supposed to be after toggling on Minerva. To fix this, change

		var labelSelector = ['vector', 'minerva'].includes(mw.config.get('skin')) ? '#pt-darkmode span' : '#pt-darkmode a';

to

		var labelSelector = ['vector', 'minerva'].includes(mw.config.get('skin')) ? '#pt-darkmode span:not(.mw-ui-icon)' : '#pt-darkmode a';

cc @SD0001 and Xaosflux. Nardog (talk) 11:00, 16 December 2021 (UTC)

 Partly done (line 31 updated) - SD, any notes on the first half of this? — xaosflux ^Talk 11:10, 16 December 2021 (UTC)

@Nardog: regarding the var removal request above, is this just some tidying? — xaosflux ^Talk 11:19, 16 December 2021 (UTC)

If p-personal is used there's no need for the conditional because it works on (AFAIK) all skins. As I noted on VPT, pt-preferences doesn't seem to exist unless the advanced mode is on, so presumably many users are not currently seeing the toggle on mobile. Nardog (talk) 11:23, 16 December 2021 (UTC)

 Done — xaosflux ^Talk 11:43, 16 December 2021 (UTC)

Line 14 dependencies

Also now that it's a gadget and the dependencies are already declared, line 14 should be simply $(function() {. Nardog (talk) 11:12, 16 December 2021 (UTC)

I personally think we should keep that. Its good to declare your dependency on module from within the code. Just because the gadget is loading it doesn't mean your code shouldn't ensure the dependencies are actually there. And if they are, then its a noop. —TheDJ (talk • contribs) 11:16, 16 December 2021 (UTC)

May also be good for scenarios where someone wants to load the script w/o using the gadget handler. — xaosflux ^Talk 11:18, 16 December 2021 (UTC)

nextnode

Xaosflux can you change the mw.util.addPortletLink line to

	var nextnode = mw.config.get('skin') === 'minerva' ? 'li:has([data-event-name="menu.unStar"])' : '#pt-watchlist';
	mw.util.addPortletLink('p-personal', '#', label, 'pt-darkmode', tooltip, '', nextnode);

currently, the toggle shows up at the end of p-personal – after the logout button, which isn't the natural position at all. The natural position would be somewhere near preferences. This puts it just before the watchlist button (which is #pt-watchlist on other skins, but on minerva there seems to be no better way to pick it other than li:has([data-event-name="menu.unStar"]). – SD0001 (talk) 14:26, 16 December 2021 (UTC)

 Done — xaosflux ^Talk 14:29, 16 December 2021 (UTC)

nextnode 2, electric boogaloo

I don't understand the need for this on Minerva. Without nextnode, the toggle already appears right beneath Settings when the advanced mode is off, and at the bottom of the personal menu when it's on (currently it appears between Sandbox and Watchlist, which I find confusing). So I would put var nextnode = mw.config.get('skin') === 'minerva' ? null : '#pt-watchlist';. Nardog (talk) 23:20, 16 December 2021 (UTC)

@Nardog: do you want to change:

var nextnode = mw.config.get('skin') === 'minerva' ? 'li:has([data-event-name="menu.unStar"])' : '#pt-watchlist';

to

var nextnode = mw.config.get('skin') === 'minerva' ? null : '#pt-watchlist';

or something else? — xaosflux ^Talk 16:29, 17 December 2021 (UTC)

No that's what I mean. Nardog (talk) 23:49, 17 December 2021 (UTC)

 Done — xaosflux ^Talk 12:06, 18 December 2021 (UTC)

Interface-protected edit request 23 December 2021

Please sync the changes from User:SD0001/dark-mode-toggle2.js (diff). The following changes have been done:

• add class client-dark-mode to <html> to allow styling via TemplateStyles or personal CSS or other gadgets
• minor optimisation of localStorage use - avoid redundant call, and wrap save call in mw.requestIdleCallback because localStorage is synchronous (blocking)

– SD0001 (talk) 14:16, 23 December 2021 (UTC)

@SD0001: Sorry to butt heads but I coded the protection against quick navigation using sessionStorage MusikAnimal and I discussed on WP:IANB here (diff with live gadget, with your sandbox). I also tidied some code: Particularly the query for <link> is now safe-guarded against external links (unlikely, I know, but it's always good to be safer); I reduced jQuery calls and renamed darkMode/inDarkMode (which I found confusing); other changes are more or less cosmetic. Merge whatever you think is an improvement if any. Nardog (talk) 16:17, 23 December 2021 (UTC)

@SD0001 and Nardog: what version needs to be promoted to this page? (Reactivate the ER when agreed!) — xaosflux ^Talk 16:19, 23 December 2021 (UTC)

Well SD is the author so feel free to promote his. I didn't mean to stall the process. Nardog (talk) 16:22, 23 December 2021 (UTC)

@Nardog thanks for that. I see your version is based on mine above, so no changes are lost.
Looking at the diff, can you undo changing $('link').attr(..., ...) to $('link', { ... }) - I think the latter can occasionally be problematic - see Krinkle's comment here. Feel free to reactivate the editreq after that. Other than that, line 79 could use Number(storageState) to avoid non-strict equality, and I wouldn't reduce jQuery usage (such as line 36 - reduces readability of code for negligible performance benefit) but anyway these are not a big deal. – SD0001 (talk) 16:58, 23 December 2021 (UTC)

Thanks for the review. The whole point of using the construction was to combine the two .attr()'s, so using .attr({}) now. Using jQuery for line 36—if IE11 had supported second argument in DOMTokenList.toggle() I wouldn't have, but given it doesn't, that's indeed neater. Nardog (talk) 17:46, 23 December 2021 (UTC)

Please merge User:Nardog/dark-mode-toggle.js (diff) per consultation with SD above. Nardog (talk) 17:46, 23 December 2021 (UTC)

 Done — xaosflux ^Talk 16:18, 25 December 2021 (UTC)

Interface-protected edit request on 30 January 2022

Please change

mw.messages.set({
	'darkmode-turn-on-label': 'Dark mode',
	'darkmode-turn-on-tooltip': 'Turn dark mode on',
	'darkmode-turn-off-label': 'Light mode',
	'darkmode-turn-off-tooltip': 'Turn dark mode off',
});

to

// Don't overwrite existing messages, if already set on a foreign wiki prior to loading this file
if (!mw.messages.get('darkmode-turn-on-label')) {
	mw.messages.set({
		'darkmode-turn-on-label': 'Dark mode',
		'darkmode-turn-on-tooltip': 'Turn dark mode on',
		'darkmode-turn-off-label': 'Light mode',
		'darkmode-turn-off-tooltip': 'Turn dark mode off',
	});
}

This makes it possible for other wikis to dynamically load dark-mode-toggle.js from here and still localise the messages. (Currently this is not possible. Using a separate JSON file for the messages also causes problems since require is not defined while loading using mw.loader.getScript.) – SD0001 (talk) 08:27, 30 January 2022 (UTC)

 Done — xaosflux ^Talk 09:53, 30 January 2022 (UTC)

Portlet icon

If someone couldn't stand the empty icon in New Vector and Minerva, they could add something like this in their CSS.

.mw-ui-icon-vector-gadget-pt-darkmode::before,
.mw-ui-icon-portletlink-pt-darkmode::before {
	background-image: url(/w/load.php?modules=oojs-ui.styles.icons-accessibility&image=moon);
}

.mw-ui-icon-portletlink-pt-darkmode::before {
	opacity: 0.65;
}

.client-dark-mode .mw-ui-icon-vector-gadget-pt-darkmode::before,
.client-dark-mode .mw-ui-icon-portletlink-pt-darkmode::before {
	background-image: url(/w/load.php?modules=oojs-ui.styles.icons-accessibility&image=bright);
}

I'm not sure if this should be part of the gadget (if so it should probably be achieved by adding the icon module as a dependency and giving the icon element the respective classes), but just noting that it could be done. I don't use either skin but the moon and the sun do make a good visual aid. Nardog (talk) 15:09, 31 January 2022 (UTC)

I like it. I left the icon empty only because I didn't know how to fix it – it does stand out like a sore thumb. – SD0001 (talk) 18:28, 31 January 2022 (UTC)

Please add the specified fragment at the top of MediaWiki:Gadget-dark-mode-toggle-pagestyles.css (before the comment). —TheDJ (talk • contribs) 08:38, 5 April 2022 (UTC)

 Done @Nardog: added, please ping me if any issues. — xaosflux ^Talk 15:10, 17 April 2022 (UTC)

This is somehow not working on mobile. I assume targets=desktop,mobile needs to be added to the gadget definition for dark-mode-toggle-pagestyles as well. (What's weird though is that it already works with useskin=vector-2022 and useskin=minerva as long as it's on the desktop site. It's as if skins=vector,monobook in the definition is taking no effect.) cc @TheDJ and MusikAnimal. Nardog (talk) 23:12, 17 April 2022 (UTC)

MW isn't even registering dark-mode-toggle-pagestyles on the mobile domain (in mw.loader.moduleRegistry). Nowadays, even the skins param (along with targets) is used for registrations, so we also need to expand the skins to include vector-2022 and minerva. – SD0001 (talk) 05:03, 18 April 2022 (UTC)

@SD0001: So is this how the definition needs to be?

* dark-mode-toggle-pagestyles[hidden|targets=desktop,mobile|skins=vector,vector-2022,minerva,monobook]|dark-mode-toggle-pagestyles.css

Nardog (talk) 06:51, 18 April 2022 (UTC)

It works fine for me in MobileFrontend, both in normal and advanced mode. What browser are you using? — MusikAnimal ^talk 23:28, 18 April 2022 (UTC)

What works fine for you? You see an icon next to the toggle and mw.loader.getState('ext.gadget.dark-mode-toggle-pagestyles') returns "ready" on the mobile site? Nardog (talk) 00:51, 19 April 2022 (UTC)

I don't know about that "ready" thing - but if you are asking if I see the icon next to "Dark mode"/"Light mode" - I do, in vector-2022 and in minerva (and they work). — xaosflux ^Talk 01:10, 19 April 2022 (UTC)

On en.m.wikipedia.org? Nardog (talk) 01:20, 19 April 2022 (UTC)

Sorry, not that was just in those skins in general, not also in MFA. — xaosflux ^Talk 10:31, 19 April 2022 (UTC)

Sorry, I just saw the ping and missed the context that this is about the icon. That indeed doesn't appear on MobileFrontend (useformat=mobile). I'm testing it now on testwiki. With the suggested gadget definition change, ext.gadget.dark-mode-toggle-pagestyles is "ready" but still no icon :( I only just made this change though so maybe ResourceLoader is still doing its thing. Or I'm missing something else. — MusikAnimal ^talk 01:27, 19 April 2022 (UTC)

You haven't updated testwiki:MediaWiki:Gadget-dark-mode-toggle-pagestyles.css yet. Nardog (talk) 01:30, 19 April 2022 (UTC)

Lol, that'll do it! Okay, icon is there now :) However it doesn't change to the 'bright' image when dark mode is on. Strangely, it looks like .client-dark-mode isn't being added to the HTML element. — MusikAnimal ^talk 01:38, 19 April 2022 (UTC)

Perhaps testwiki:MediaWiki:Gadget-dark-mode-toggle.js needs to be synced as well. Nardog (talk) 01:41, 19 April 2022 (UTC)

Maybe I should go to bed. Thanks, um, for holding my ḣand throughout this. All done :) — MusikAnimal ^talk 01:49, 19 April 2022 (UTC)

Edit request 21 April 2022

Toggling more than once on the same page on mobile somehow didn't keep the icon for me, and turns out the script (line 66) was removing dark-mode from ext.gadget.dark-mode-toggle-pagestyles, leaving a link to /w/load.php?lang=en&modules=ext.gadget.-toggle-pagestyles&only=styles&skin=minerva. Here's what I came up with to fix this. @SD0001: can you take a look? Nardog (talk) 02:59, 19 April 2022 (UTC)

@MusikAnimal and TheDJ: Or maybe you can... Nardog (talk) 02:10, 21 April 2022 (UTC)

LGTM. Thanks for fixing. – SD0001 (talk) 05:23, 21 April 2022 (UTC)

Thanks for the review. Please copy from User:Nardog/dark-mode-toggle.js (diff). Nardog (talk) 06:34, 21 April 2022 (UTC)

 Done — MusikAnimal ^talk 16:52, 25 April 2022 (UTC)

Modify Minerva theme color

Minerva sets a fixed theme-color, which overrides the automatic theme-color (background color of body). This theme color is used by PWA and iOS Safari. This change will add a small change to the dark toggle gadget to adapt this color via javascript. Its less than ideal, but its a start. —TheDJ (talk • contribs) 15:16, 3 February 2022 (UTC)

Line 29 is already in an if block, perhaps you don't need the ternary? Nardog (talk) 16:27, 3 February 2022 (UTC)

I guess, done. —TheDJ (talk • contribs) 12:57, 4 February 2022 (UTC)

 Done Synced - revert if issues. — xaosflux ^Talk 14:55, 9 February 2022 (UTC)

Skin-agnostic label toggling

Now that the new Vector is its own skin, we could simply add "vector-2022" to the labelSelector condition, but I have another idea. Since skins and the way they make portlet links can change all the time, what if we just looked for and changed the text node for the label directly? As in:

$('#pt-darkmode, #pt-darkmode *').contents().each(function () {
	// If it's a text node and has more than just whitespace
	if (this.nodeType === 3 && /\S/.test(this.textContent)) {
		this.textContent = mw.msg('darkmode-turn-' + onOrOff + '-label');
		return false; // break
	}
});

That way the script won't make assumptions about each skin so the toggling will (likely) always work even if skins change the ways they make portlet links. Nardog (talk) 07:36, 4 February 2022 (UTC)

That seems like an extremely expensive Dom operation to me to do on all pages. —TheDJ (talk • contribs) 12:56, 4 February 2022 (UTC)

But it won't be done on all pages, only when you toggle. And the loop won't be long as a portlet container is not going to have more than a few nodes anyway. If it's the * selector that's expensive I guess we could use $('#pt-darkmode').find('*').addBack().contents().each(...) instead, though I don't see a difference on my end. Nardog (talk) 17:13, 4 February 2022 (UTC)

@SD0001: What do you think? We need to support Vector 2022 one way or another (currently the label gets bigger after toggling). If you don't like this idea either we should simply add it to the array. Nardog (talk) 15:34, 6 February 2022 (UTC)

I think let's just add it to the array. Not sure of being expensive, but it still looks rather hacky -- too bad there's no supported way to change the portlet link text. – SD0001 (talk) 15:14, 9 February 2022 (UTC)

Well neither jQuery nor DOM have handy interfaces for manipulating text nodes, so this is about as simple as it gets if we want to have a future-proof way of toggling the label (one might be able to make it look a little simpler, e.g. by using .filter() or .get().find(), but that's hardly so). Nardog (talk) 09:45, 10 February 2022 (UTC)

Edit request 17 April 2022

Please update from User:Nardog/dark-mode-toggle.js (diff). It includes the fix for the Vector 2022 split (see correspondence with SD0001 above), minor refactoring in whitespace, and a minor correction in comments (theme-color is present on all mobile pages regardless of skin). Nardog (talk) 23:25, 17 April 2022 (UTC)

 Done — xaosflux ^Talk 10:43, 18 April 2022 (UTC)

Interface-protected edit request on 15 October 2022

Please update JS from here (diff) and CSS from here (diff). This will allow the toggle to appear in Vector 2022's sticky header as well. See this discussion for context and consultation with SD0001, the script author. Nardog (talk) 10:54, 15 October 2022 (UTC)

Nardog -  Done. ~Oshwah~^{(talk) (contribs)} 13:07, 31 October 2022 (UTC)

@Nardog this doesn't seem to reliably work. The option doesn't show up most of the time, because the gadget code executes too early – likely before the sticky header is even generated. – SD0001 (talk) 19:42, 4 November 2022 (UTC)

Please sync from testwiki:MediaWiki:Gadget-dark-mode-toggle.js (diff). This ensures the portlet link within sticky header is added only after the skins.vector.es6 module (which creates the sticky header) has loaded. – SD0001 (talk) 19:45, 4 November 2022 (UTC)

@SD0001: In that case, the event handler should also be attached after the skin module (or use delegation and attach it to document.body or smth). Nardog (talk) 20:45, 4 November 2022 (UTC)

@Nardog: Fixed. Thanks for pointing that out. Also had to re-evaluate label and tooltip before adding the new portlet, since in some cases (such as when state is recovered from sessionStorage) the labels in the normal and sticky headers were different. – SD0001 (talk) 13:20, 5 November 2022 (UTC)

Not sure if something changed in vector since the above posts, but now this is no longer working (#p-personal-sticky-header is not present on DOM even after skins.vector.es6 has loaded). – SD0001 (talk) 06:22, 19 November 2022 (UTC)

@SD0001: It looks like it's now p-personal-content-container-sticky-header. Can you see if simply changing the ID works on testwiki? (If not also .children() while we're at it.) Nardog (talk) 11:26, 19 November 2022 (UTC)

addPortletLink call works fine with either portlet ID after the sticky header has appeared. But I'm not sure how to wait for it to appear, without resorting to setTimeout or MutationObserver (awaiting skins.vector.es6 module had worked earlier). – SD0001 (talk) 19:09, 19 November 2022 (UTC)

@SD0001: How about this? Nardog (talk) 02:55, 20 November 2022 (UTC)

@Nardog Great. Another problem showed up however – no icon in the sticky header! .mw-ui-icon-vector-gadget-pt-darkmode-sticky-header which the peer gadget targets doesn't get automatically added anymore. Had to resolve that with this edit. – SD0001 (talk) 07:12, 20 November 2022 (UTC)

@SD0001: That almost feels like it shouldn't be the case... I would do $('#pt-darkmode-sticky-header span:only-child').before(...); to be future-proof in case it gets fixed. Nardog (talk) 01:06, 21 November 2022 (UTC)

Edit request 26 October 2022

Description of suggested change: please add .skin-vector-2022 .vector-user-menu-logged-out #pt-darkmode .mw-ui-icon{display:none}.skin-vector-2022 .vector-user-menu-logged-out #pt-darkmode span{margin-left:0;margin-right:0} to MediaWiki:Gadget-dark-mode-toggle-pagestyles.css. This removes the icon+indentation for logged out Vector-2022 users. (Vector-2022 logged-out menu has no icons) — Alexis Jazz (talk or ping me) 09:49, 26 October 2022 (UTC)

Alexis Jazz -  Done. ~Oshwah~^{(talk) (contribs)} 13:18, 31 October 2022 (UTC)

Rewrite

I think the code has become rather messy with the recent addition of new features. I have attempted a rewrite that organises everything into functions, for hopefully better readability and avoidance of race conditions – see User:SD0001/dark-mode-toggle2.js. @Nardog Thoughts? – SD0001 (talk) 13:34, 5 November 2022 (UTC)

Looks good. I was also wondering if nextnode needs to be a variable at all. It's ignored and added to the bottom if #pt-watchlist isn't found anyway. I'm also not sure if we need the entire script to wait for $.ready. Thus here's my suggestions. Nardog (talk) 14:06, 5 November 2022 (UTC)

The toggleMode calls in line 148/154/159 would manipulate portlets, so I believe should wait for $.ready. – SD0001 (talk) 15:26, 5 November 2022 (UTC)

True. What about nextnode? Nardog (talk) 01:58, 6 November 2022 (UTC)

Yeah that's fine. Have deployed changes on testwiki:MediaWiki:Gadget-dark-mode-toggle.js. Let's see if any bug shows up. – SD0001 (talk) 09:12, 10 November 2022 (UTC)

@SD0001: One tiny thing I don't like using the toggle with legacy Vector (but also applicable to Timeless) is that it toggles even if you click on the portlet margin. How about .children() to L50? Nardog (talk) 13:51, 11 November 2022 (UTC)

I see that on timeless. On legacy vector, I find it hardly discernible as the li and a basically cover the same area. Wouldn't adding .children() cause an issue with vector-2022 in which the whole of the li is supposed to be clickable? – SD0001 (talk) 19:19, 11 November 2022 (UTC)

It wouldn't because the whole of the li is made clickable by making the a element fill it. Nardog (talk) 14:47, 12 November 2022 (UTC)

@SD0001: Do we have to keep mw.user.options.set()? Nardog (talk) 01:12, 21 November 2022 (UTC)

It's also kind of weird that we're (unsafely) using BroadcastChannel, which Safari started supporting less than a year ago, but also es6-polyfills... (And labelSelector may be better rewritten with a switch statement anyway.) Nardog (talk) 09:54, 21 November 2022 (UTC)

Yeah the es6-polyfills dependency is superfluous considering that the dark-mode CSS itself is unlikely to work on pre-ES6 browsers. As for mw.user.options, I suppose it doesn't hurt? – SD0001 (talk) 15:18, 19 January 2023 (UTC)

@SD0001: Time to deploy now that Vector 2022 has been rolled out? As always I have suggested code at User:Nardog/dark-mode-toggle.js. Nardog (talk) 14:55, 19 January 2023 (UTC)

Sure – I had forgotten about this completely. – SD0001 (talk) 15:17, 19 January 2023 (UTC)

Does mine look good for you or should we go with yours on testwiki? Nardog (talk) 16:03, 19 January 2023 (UTC)

Yeah it looks good, though I haven't tested. – SD0001 (talk) 16:33, 19 January 2023 (UTC)

Please copy from User:Nardog/dark-mode-toggle.js. It includes fix for the Vector 2022 sticky header. Nardog (talk) 04:34, 20 January 2023 (UTC)

Then, remove ,es6-polyfills from the line

* dark-mode-toggle[ResourceLoader|targets=desktop,mobile|dependencies=mediawiki.util,mediawiki.api,mediawiki.Uri,mediawiki.storage,es6-polyfills|peers=dark-mode-toggle-pagestyles]|dark-mode-toggle.js

Nardog (talk) 04:34, 20 January 2023 (UTC)

 Done  Undone @Nardog: this seem to create an error where the control would say you were in light mode when in dark, and the reverse. — xaosflux ^Talk 14:23, 26 January 2023 (UTC)

@Xaosflux: Sorry, should be fixed now. Nardog (talk) 14:29, 26 January 2023 (UTC)

re-enqueued — xaosflux ^Talk 14:31, 26 January 2023 (UTC)

Deployed this on testwiki. Looks good to me after testing across different skins. – SD0001 (talk) 11:29, 28 January 2023 (UTC)

 Done — xaosflux ^Talk 12:02, 28 January 2023 (UTC)

Minerva fix

The toggle icon has been glitchy on Minerva for a while. Please apply the patch here and here. Nardog (talk) 06:22, 1 December 2023 (UTC)

Nardog I've done this change, but since you mention Minerva specifically, that line is fallthrough from Vector/2022 as well. Did you mean to impact those? Izno (talk) 22:47, 1 December 2023 (UTC)

Yes. span:not(:empty) works for all three skins. Nardog (talk) 23:49, 1 December 2023 (UTC)

Edit request 16 May 2024

@Jon (WMF): There is a typo in the message at line 131:

Diff:

−

mw.notify( 'Native dark mode has been enabled. Please set "Color" to light theme to avoid conflicts with the dark mode gadget to supress this message.' );

+

mw.notify( 'Native dark mode has been enabled. Please set "Color" to light theme to avoid conflicts with the dark mode gadget to suppress this message.' );

ZandDev 16:59, 16 May 2024 (UTC)

 Done Dealt with. ~ Amory (u • t • c) 17:39, 16 May 2024 (UTC)

Thanks! Jon (WMF) (talk) 18:33, 16 May 2024 (UTC)

Interface-protected edit request on 16 May 2024

@Jon (WMF): Please revert the recent changes to the gadget.

As seen in this screen recording, the warning is showing up on every page even when Color is set to Light. I get this issue on multiple devices. There's no way to get the popup to go away.

The notice really needs to be shown only when the gadget's dark mode and native dark mode are both active simultaneously. – SD0001 (talk) 17:45, 16 May 2024 (UTC)

@SD0001 hey there - I've updated the message to include a link and to be dismissed once you've selected "day" theme explicitly. This also disables the interface. I hope this resolves the issue.

If not, open to new ideas. Jon (WMF) (talk) 18:33, 16 May 2024 (UTC)

It doesn't solve the issue. I'm already on the "day" theme and still see the message. Also, the radios to change the theme are greyed out – on some pages it says "This page is always in light mode" while on others it says "This has been disabled by the dark mode gadget. Disable the gadget to use this feature."

This gadget has 87K users. Please test the changes on testwiki:MediaWiki:Gadget-dark-mode-toggle.js before deploying them here. – SD0001 (talk) 20:13, 16 May 2024 (UTC)

I would also note, @Jon (WMF), that this gadget is loaded in other wikis (e. g. ru:MediaWiki:Gadget-dark-mode-toggle.js) and the changes 1) made it impossible to localise part of the messages in it, and 2) made it impossible to debug the WMF night mode without disabling the gadget entirely. stjn 16:12, 19 May 2024 (UTC)

• @SD0001, Jon (WMF), and Stjn: is the request here to revert back to the 2023-12-01 revision? Staffers, please note that a private Discord conversation isn't good documentation or a way to build consensus for a community maintained script. — xaosflux ^Talk 16:20, 19 May 2024 (UTC)

  Yes. – SD0001 (talk) 16:39, 19 May 2024 (UTC)

   Done * Pppery * _{it has begun...} 15:18, 20 May 2024 (UTC)

  @Pppery Might be nice to note having done so at phab:T365083 ~ Amory (u • t • c) 16:44, 20 May 2024 (UTC)

  Done. But you have just as much access to Phabricator as I do and could have done it yourself. * Pppery * _{it has begun...} 16:45, 20 May 2024 (UTC)

  Fair! Didn't want to seem like tattling or presuming to speak for you; apologies. ~ Amory (u • t • c) 16:50, 20 May 2024 (UTC)

  Reverting the changes leaves the gadget in a broken state, leaving users with an unreadable/unusable page with no explanation how to fix it. This seems a worse state to me, as "annoying" seems better than "unusable" but that's just two cents. FWIW the reason this was rushed from Discord was there were several reports I was alerted to on Reddit, Discord and Twitter where people couldn't read the site which is bad.

  My priority/focus right now is to support updating the skin to detect and disable itself with the gadget (tracked in phab:T365083) after which we can safely revert these changes and restore the gadget back to the default state. Hopefully this can be done in 1-3 weeks. Jon (WMF) (talk) 17:03, 20 May 2024 (UTC)

  Can you explain what is "broken"? I have this gadget enabled, and it is working fine for me. — xaosflux ^Talk 18:50, 20 May 2024 (UTC)

  @Jon (WMF) assuming it is only broken for someone who may have opted in to the beta feature, while also opting in to this gadget (their choice)? — xaosflux ^Talk 18:55, 20 May 2024 (UTC)

  Correct. Jon (WMF) (talk) 19:50, 22 May 2024 (UTC)

  With the distinction that many people do not remember they opted into a gadget and how to disable it (e.g. they following a Reddit post one day and forgot about it) Jon (WMF) (talk) 19:50, 22 May 2024 (UTC)

  Basically, people with dark mode via gadget enabled and reading feature enabled get a hyper-white theme because of the way gadget-based dark mode works (CSS filters). (I don't think the edits in question should’ve been reverted, just iterated.) stjn 22:57, 20 May 2024 (UTC)

  While someone determines what the best next step is, perhaps we should warn at MediaWiki:Vector-2022-beta-preview-description that these features may not be compatible with scripts and gadgets (such as the dark mode gadget)? — xaosflux ^Talk 22:54, 22 May 2024 (UTC)

Edit request 12 October 2024

See here. I don't get it:

• Sometimes it looks for ext.gadget.dark-mode and sometimes for dark-mode…
• Not to mention the terrible .replace('ext.gadget.dark-mode,', 'ext.gadget.') replacement (so the next entry would become "ext.gadget.ext.gadget.foobar"…)

Current code:

		if (isOn) {
			uri.query.modules += ',dark-mode';
		} else {
			if (uri.query.modules === 'ext.gadget.dark-mode') {
				// dark-mode is the only module in this link
				$gadgetsLink.remove();
				return;
			}
			uri.query.modules = uri.query.modules
				.replace('ext.gadget.dark-mode,', 'ext.gadget.') // dark-mode is first in the gadget list
				.replace(/,dark-mode(,|$)/, '$1'); // dark-mode is in middle or end of the list
		}

Presumably fixed code (not tested at all):

		if (isOn) {
			uri.query.modules += ',ext.gadget.dark-mode';
		} else {
			if (uri.query.modules === 'ext.gadget.dark-mode') {
				// dark-mode is the only module in this link
				$gadgetsLink.remove();
				return;
			}
			uri.query.modules = uri.query.modules
				.replace('ext.gadget.dark-mode,', '') // dark-mode is first in the gadget list
				.replace(/,ext.gadget.dark-mode(,|$)/, '$1'); // dark-mode is in middle or end of the list
		}

Od1n (talk) 07:31, 12 October 2024 (UTC)

the next entry would become "ext.gadget.ext.gadget.foobar" That's not what I'm seeing. The link element is like <link rel="stylesheet" href="/w/load.php?lang=en&amp;modules=ext.gadget.Twinkle-pagestyles%2Cdark-mode%2Cdark-mode-toggle-pagestyles&amp;only=styles&amp;skin=vector-2022"> so the current code correctly removes just dark-mode and one %2C. Have you actually found the toggle not working? Nardog (talk) 07:43, 12 October 2024 (UTC)

Not tried at all. Just noticed this when removing uses of the deprecated mw.Uri on local forks (here and here). So, would you confirm the current code is correct? I'm still doubting, note how the two "replace()" look for different texts… Od1n (talk) 07:49, 12 October 2024 (UTC)

They do what the comments say.

// first in the list
'ext.gadget.dark-mode,dark-mode-toggle-pagestyles,Twinkle-pagestyles' 
	.replace('ext.gadget.dark-mode,', 'ext.gadget.')
	.replace(/,dark-mode(,|$)/, '$1');
// -> 'ext.gadget.dark-mode-toggle-pagestyles,Twinkle-pagestyles'
// i.e. 'dark-mode,' removed

// middle in the list
'ext.gadget.Twinkle-pagestyles,dark-mode,dark-mode-toggle-pagestyles'
	.replace('ext.gadget.dark-mode,', 'ext.gadget.')
	.replace(/,dark-mode(,|$)/, '$1');
// -> 'ext.gadget.Twinkle-pagestyles,dark-mode-toggle-pagestyles'
// i.e. 'dark-mode,' removed

// last in the list
'ext.gadget.Twinkle-pagestyles,dark-mode-toggle-pagestyles,dark-mode'
	.replace('ext.gadget.dark-mode,', 'ext.gadget.')
	.replace(/,dark-mode(,|$)/, '$1');
// -> 'ext.gadget.Twinkle-pagestyles,dark-mode-toggle-pagestyles'
// i.e. ',dark-mode' removed

Please test your code before requesting an edit. Nardog (talk) 08:08, 12 October 2024 (UTC)

Understood, the "ext.gadget." prefix is shared. I was wrongly assuming "ext.gadget.foo,ext.gadget.bar"…

Just for inspiration, a different way of making the replacement:

uri.query.modules = uri.query.modules
    .replace(/ext\.gadget\.([^|]+)/g, (match, p1) =>
        'ext.gadget.' + p1
            .split(',')
            .filter(value => value !== 'dark-mode')
            .join(',')
    );

And I agree with your comment (that you removed), these local code forks add a lot of maintenance burden. Not my fault… Od1n (talk) 12:56, 12 October 2024 (UTC)

A more elaborate version, to avoid error if the link has only the "dark-mode" gadget, but also non-gadget modules (e.g. "ext.gadget.dark-mode|foobar"):

uri.query.modules = uri.query.modules
    .split('|')
    .map(value => value.replace(/^ext\.gadget\.(.+)$/, (match, p1) =>
        'ext.gadget.' + p1
            .split(',')
            .filter(value => value !== 'dark-mode')
            .join(',')
    ))
    .filter(value => value !== 'ext.gadget.')
    .join('|');

Od1n (talk) 13:15, 12 October 2024 (UTC)

Does MediaWiki ever serve a link element with gadget and non-gadget modules? Nardog (talk) 00:38, 13 October 2024 (UTC)

Never on Wikimedia sites. Modules can be served together when they share the same ResourceLoader group. All styles-only gadgets are in the "site" group, which is not used by core modules or extensions. ext.globalCssJs.site.styles is also in this group but is disabled on Wikimedia cluster. – SD0001 (talk) 20:51, 15 October 2024 (UTC)