Dark mode

All suggestions about TPFC should be posted here. Discussions about changes to TPFC will also be carried out here.
Message
Author
lwc
Posts: 284
Joined: Tue Jun 26, 2012 10:40 pm
Contact:

Re: Dark mode

#46 Post by lwc »

I know why - this phpBB extension also has a PHP part that just assumes if the cookie exists, then surely one wants dark mode. But now that we save a cookie even if it's false, the PHP part must be updated too.
To do that, in the extension's listener.php change:

Code: Select all

		$dark_case = $this->request->variable($cookie_darkmode_name, false, false, \phpbb\request\request_interface::COOKIE);
to:

Code: Select all

		$dark_case = $this->request->variable($cookie_darkmode_name, false, false, \phpbb\request\request_interface::COOKIE) === 'true';
Since this is server-side, it can't be simulated beforehand, so let us know once you did it.

User avatar
Andrew Lee
Posts: 3095
Joined: Sat Feb 04, 2006 9:19 am
Contact:

Re: Dark mode

#47 Post by Andrew Lee »

@lwc: Updated! I briefly tried it out in a Chrome private window, and it seems to work. Thanks!

lwc
Posts: 284
Joined: Tue Jun 26, 2012 10:40 pm
Contact:

Re: Dark mode

#48 Post by lwc »

Unfortunately it doesn't seem to work - because it keeps switching to light mode now (the opposite of what happened previously).
Can you attach your listener.php post the modifications so I will compare it to the original file?

User avatar
Andrew Lee
Posts: 3095
Joined: Sat Feb 04, 2006 9:19 am
Contact:

Re: Dark mode

#49 Post by Andrew Lee »

Here it is:

Code: Select all

<?php
/**
 *
 * Dark Mode extension for the phpBB Forum Software package.
 *
 * @copyright (c) 2013 phpBB Limited <https://www.phpbb.com>
 * @license GNU General Public License, version 2 (GPL-2.0)
 *
 */

namespace aurelienazerty\darkmode\event;

/**
 * Event listener
 */
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

class listener implements EventSubscriberInterface
{
	/** @var \phpbb\template\template */
	protected $template;
	/** @var \phpbb\user $user */
	protected $user;
	/** @var \phpbb\request\request */
	protected $request;
	/** @var \phpbb\config\config */
	protected $config;


	/**
	 * Constructor
	 *
	 * @param \phpbb\template\template	$template Template object
	 * @param \phpbb\user					$user User object
	 * @param \phpbb\request\request		$request Request object
	 * @param \phpbb\config\config			$config	Config object
	 * @return \aurelienazerty\darkmode\event\listener
	 * @access public
	 */
	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\request\request $request,\phpbb\config\config $config)
	{
		$this->template = $template;
		$this->user 	= $user;
		$this->request 	= $request;
		$this->config	= $config;
	}

	static public function getSubscribedEvents()
	{
		return array(
			'core.page_header'				=> 'do_dark',
			'core.adm_page_header'		=> 'do_dark',
		);
	}

	public function do_dark($event)
	{
		$cookie_darkmode_name = $this->config['cookie_name'] . '_darkmode';

#		$dark_case = $this->request->variable($cookie_darkmode_name, false, false, \phpbb\request\request_interface::COOKIE);
		$dark_case = $this->request->variable($cookie_darkmode_name, false, false, \phpbb\request\request_interface::COOKIE) === 'true';
		if ($dark_case)
		{
			$style_do_light = "";
			$style_do_dark = "display: none;";
			$class = "darkmode";
		}
		else
		{
			$style_do_light = "display: none;";
			$style_do_dark = "";
			$class = "lightmode";
		}

		$this->user->add_lang_ext('aurelienazerty/darkmode', 'dark_mode');

		$this->template->assign_vars(array(
			'S_DARKMODE_ROOT_CLASS'				=> 	$class,
			'STYLE_DO_DARK'						=> 	$style_do_dark,
			'STYLE_DO_LIGHT'					=> 	$style_do_light,
			'DO_DARK_MESSAGE'					=> 	$this->user->lang['DO_DARK_MODE'],
			'DO_LIGHT_MESSAGE'				=> 	$this->user->lang['DO_LIGHT_MODE'],
			'S_COOKIE_DARKMODE_NAME'	=>	$cookie_darkmode_name,
		));
	}
}

User avatar
Andrew Lee
Posts: 3095
Joined: Sat Feb 04, 2006 9:19 am
Contact:

Re: Dark mode

#50 Post by Andrew Lee »

OK, I think I found the problem. The correct code should be:

Code: Select all

$dark_case = $this->request->variable($cookie_darkmode_name, [b]'false'[/b], false, \phpbb\request\request_interface::COOKIE) == 'true';
(Note the second parameter [default cookie value] should be a string, not a boolean, since cookie values are stored as strings).

I have now tested the hell out of the this change, so if it still doesn't work, it just proves I don't have the talent to be a tester! :D

lwc
Posts: 284
Joined: Tue Jun 26, 2012 10:40 pm
Contact:

Re: Dark mode

#51 Post by lwc »

It indeed works now, thanks!
But that false is only a default in case there's no cookie, while our issue was not reading the existing cookie correctly.
Besides, === (which for some reason you changed to ==) made sure comparing false to 'true' would not match.
Since for an unknown reason it fixed it, I wouldn't change anything. But I am curious as to why changing false to 'false' and === and == seemingly fixed things.

User avatar
Andrew Lee
Posts: 3095
Joined: Sat Feb 04, 2006 9:19 am
Contact:

Re: Dark mode

#52 Post by Andrew Lee »

lwc wrote: Wed Jun 19, 2024 10:22 pm It indeed works now, thanks!
But that false is only a default in case there's no cookie, while our issue was not reading the existing cookie correctly.
Besides, === (which for some reason you changed to ==) made sure comparing false to 'true' would not match.
Since for an unknown reason it fixed it, I wouldn't change anything. But I am curious as to why changing false to 'false' and === and == seemingly fixed things.
After more investigation, I think I have the answer to your question.

The phpbb API says:
Return Value
mixed
The value of $_REQUEST[$var_name] run through set_var set_var to ensure that the type is the the same as that of $default. If the variable is not set $default is returned.
So if the default value is boolean, the return value is also boolean, so it is always 1 (or true), since the string 'true' or 'false' are both converted to boolean true. So the subsequent match is always false.

However, if we change the default value to a string, then the value of the cookie is returned as a string. Whether "==" or "===" is used becomes inconsequential here. I have changed to "===" with the same result.

lwc
Posts: 284
Joined: Tue Jun 26, 2012 10:40 pm
Contact:

Re: Dark mode

#53 Post by lwc »

Good catch! Overall cookie matching should be done strictly in client side (i.e. JS), not server side like PHP, because only client side can detect the OS' dark mode setting.
I'm surprised in all these years no one cared enough to produce a more efficient extension, maybe I will some time.

User avatar
Andrew Lee
Posts: 3095
Joined: Sat Feb 04, 2006 9:19 am
Contact:

Re: Dark mode

#54 Post by Andrew Lee »

I received a private email from ripu about the recently implemented dark mode. However, I think this issue is best discussed in the forum, so I am reproducing his email below:
Hi,

The dark mode is too bad. It's 0 dark with 255 text. Which harts eyes, like a flash light in your eyes in a dark room. It should not be like that. Look at google or any other big app's dark mode. Links are also too blue for dark mode.

Please reduce the contrast to something better. e.g.
background colour: #222222
text colour: #cccccc
link colour: #6bbddd

In addition please remove the transparent image from tooltips, this one:
p.tooltip > background: https://www.portablefreeware.com/images/groovepaper.png

It creates a white background in star popularity ratings when hovering over in dark mode.

This will be much better for our eyes. Please. Begging you.

Thanks
As such, I have updated the dark mode CSS to:

Code: Select all

@media (prefers-color-scheme: dark) {
	:not(a), input::placeholder {color: #cccccc !important; background-color: #222222 !important;}
	a:link:not(.username-coloured) {color: #6bbddd !important;}
	a:visited:not(.username-coloured) {color: pink !important;}
	a:hover:not(.username-coloured) {color: yellow !important;}
	fieldset>input[type="submit"], fieldset>input[type="reset"] {background-image: initial !important;}
	option:checked, [role="listbox"] *:hover, p>input[type="submit"],div>input[type="submit"] {background-color: gray !important;}
	.ui-datepicker-calendar * {background: initial !important;}
	p.tooltip {background: #222222 !important;}
}
If you encounter any issues with this change, please let me know.

User avatar
joby_toss
Posts: 2982
Joined: Sat Feb 09, 2008 9:57 am
Location: Romania
Contact:

Re: Dark mode

#55 Post by joby_toss »

Better now, thanks!

User avatar
Midas
Posts: 6790
Joined: Mon Dec 07, 2009 7:09 am
Location: Sol3

Re: Dark mode

#56 Post by Midas »

Less stark now, for sure. 8)

Post Reply