From 61f401da57d6d883dbee2b236f7c2f77e7fa9612 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 14 Dec 2023 17:56:42 +0100 Subject: [PATCH] fix(theming): Adjust hover color to be accessible Signed-off-by: Ferdinand Thiessen --- apps/theming/css/default.css | 2 +- apps/theming/lib/Themes/CommonThemeTrait.php | 2 +- .../tests/Themes/AccessibleThemeTestCase.php | 116 ++++++++++++++++++ .../theming/tests/Themes/DefaultThemeTest.php | 29 ++--- 4 files changed, 131 insertions(+), 18 deletions(-) create mode 100644 apps/theming/tests/Themes/AccessibleThemeTestCase.php diff --git a/apps/theming/css/default.css b/apps/theming/css/default.css index 17b3eea8381..4119d370f08 100644 --- a/apps/theming/css/default.css +++ b/apps/theming/css/default.css @@ -77,7 +77,7 @@ --color-primary-light-text: #00293f; --color-primary-light-hover: #dbe4ea; --color-primary-element: #00679e; - --color-primary-element-hover: #1674a6; + --color-primary-element-hover: #1070a4; --color-primary-element-text: #ffffff; --color-primary-element-text-dark: #f0f0f0; --color-primary-element-light: #e5eff5; diff --git a/apps/theming/lib/Themes/CommonThemeTrait.php b/apps/theming/lib/Themes/CommonThemeTrait.php index 0b033b3fac9..38ca760db9f 100644 --- a/apps/theming/lib/Themes/CommonThemeTrait.php +++ b/apps/theming/lib/Themes/CommonThemeTrait.php @@ -66,7 +66,7 @@ trait CommonThemeTrait { // used for buttons, inputs... '--color-primary-element' => $colorPrimaryElement, - '--color-primary-element-hover' => $this->util->mix($colorPrimaryElement, $colorMainBackground, 82), + '--color-primary-element-hover' => $this->util->mix($colorPrimaryElement, $colorMainBackground, 87), '--color-primary-element-text' => $this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff', // mostly used for disabled states '--color-primary-element-text-dark' => $this->util->darken($this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff', 6), diff --git a/apps/theming/tests/Themes/AccessibleThemeTestCase.php b/apps/theming/tests/Themes/AccessibleThemeTestCase.php new file mode 100644 index 00000000000..6c31506999d --- /dev/null +++ b/apps/theming/tests/Themes/AccessibleThemeTestCase.php @@ -0,0 +1,116 @@ + + * + * @author Ferdinand Thiessen + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\Theming\Tests\Themes; + +use OCA\Theming\ITheme; +use OCA\Theming\Util; +use Test\TestCase; + +class AccessibleThemeTestCase extends TestCase { + protected ITheme $theme; + protected Util $util; + + public function dataAccessibilityPairs() { + return [ + 'primary-element on background' => [ + [ + '--color-primary-element', + '--color-primary-element-hover', + ], + [ + '--color-main-background', + '--color-background-hover', + '--color-background-dark', + '--color-background-darker', + '--color-main-background-blur', + ], + 3.0, + ], + 'primary-element-text' => [ + ['--color-primary-element-text'], + [ + '--color-primary-element', + '--color-primary-element-hover', + ], + 4.5, + ], + 'primary-element-light-text' => [ + ['--color-primary-element-light-text'], + [ + '--color-primary-element-light', + '--color-primary-element-light-hover', + ], + 4.5, + ], + 'main-text' => [ + ['--color-main-text'], + [ + '--color-main-background', + '--color-background-hover', + '--color-background-dark', + '--color-background-darker', + '--color-main-background-blur', + ], + 4.5, + ], + 'max-contrast-text' => [ + ['--color-text-maxcontrast'], + [ + '--color-main-background', + '--color-background-hover', + '--color-background-dark', + ], + 4.5, + ], + 'max-contrast text-on blur' => [ + ['--color-text-maxcontrast-background-blur'], + [ + '--color-main-background-blur', + ], + 4.5, + ], + ]; + } + + /** + * @dataProvider dataAccessibilityPairs + */ + public function testAccessibilityOfVariables($mainColors, $backgroundColors, $minContrast) { + $this->assertNotNull($this->theme, 'You need to setup $this->theme in your setUp function'); + $this->assertNotNull($this->util, 'You need to setup $this->util in your setUp function'); + + $variables = $this->theme->getCSSVariables(); + + // Blur effect does not work so we mockup the color - worst supported case is the default "clouds" background image (on dark themes the clouds with white color are bad on bright themes the primary color as sky is bad) + $variables['--color-main-background-blur'] = $this->util->mix($variables['--color-main-background'], $this->util->isBrightColor($variables['--color-main-background']) ? $variables['--color-primary'] : '#ffffff', 75); + + foreach ($backgroundColors as $background) { + $this->assertStringStartsWith('#', $variables[$background], 'Is not a plain color variable - consider to remove or fix this test'); + foreach ($mainColors as $main) { + $this->assertStringStartsWith('#', $variables[$main], 'Is not a plain color variable - consider to remove or fix this test'); + $realContrast = $this->util->colorContrast($variables[$main], $variables[$background]); + $this->assertGreaterThanOrEqual($minContrast, $realContrast, "Contrast is not high enough for $main (" . $variables[$main] . ") on $background (" . $variables[$background] . ')'); + } + } + } +} diff --git a/apps/theming/tests/Themes/DefaultThemeTest.php b/apps/theming/tests/Themes/DefaultThemeTest.php index 0d86a8d6b28..7e823cb4843 100644 --- a/apps/theming/tests/Themes/DefaultThemeTest.php +++ b/apps/theming/tests/Themes/DefaultThemeTest.php @@ -20,7 +20,7 @@ * along with this program. If not, see . * */ -namespace OCA\Theming\Tests\Service; +namespace OCA\Theming\Tests\Themes; use OCA\Theming\AppInfo\Application; use OCA\Theming\ImageManager; @@ -36,9 +36,8 @@ use OCP\IL10N; use OCP\IURLGenerator; use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; -use Test\TestCase; -class DefaultThemeTest extends TestCase { +class DefaultThemeTest extends AccessibleThemeTestCase { /** @var ThemingDefaults|MockObject */ private $themingDefaults; /** @var IUserSession|MockObject */ @@ -54,8 +53,6 @@ class DefaultThemeTest extends TestCase { /** @var IAppManager|MockObject */ private $appManager; - private DefaultTheme $defaultTheme; - protected function setUp(): void { $this->themingDefaults = $this->createMock(ThemingDefaults::class); $this->userSession = $this->createMock(IUserSession::class); @@ -65,7 +62,7 @@ class DefaultThemeTest extends TestCase { $this->l10n = $this->createMock(IL10N::class); $this->appManager = $this->createMock(IAppManager::class); - $util = new Util( + $this->util = new Util( $this->config, $this->appManager, $this->createMock(IAppData::class), @@ -101,8 +98,8 @@ class DefaultThemeTest extends TestCase { return "/$app/img/$filename"; }); - $this->defaultTheme = new DefaultTheme( - $util, + $this->theme = new DefaultTheme( + $this->util, $this->themingDefaults, $this->userSession, $this->urlGenerator, @@ -117,31 +114,31 @@ class DefaultThemeTest extends TestCase { public function testGetId() { - $this->assertEquals('default', $this->defaultTheme->getId()); + $this->assertEquals('default', $this->theme->getId()); } public function testGetType() { - $this->assertEquals(ITheme::TYPE_THEME, $this->defaultTheme->getType()); + $this->assertEquals(ITheme::TYPE_THEME, $this->theme->getType()); } public function testGetTitle() { - $this->assertEquals('System default theme', $this->defaultTheme->getTitle()); + $this->assertEquals('System default theme', $this->theme->getTitle()); } public function testGetEnableLabel() { - $this->assertEquals('Enable the system default', $this->defaultTheme->getEnableLabel()); + $this->assertEquals('Enable the system default', $this->theme->getEnableLabel()); } public function testGetDescription() { - $this->assertEquals('Using the default system appearance.', $this->defaultTheme->getDescription()); + $this->assertEquals('Using the default system appearance.', $this->theme->getDescription()); } public function testGetMediaQuery() { - $this->assertEquals('', $this->defaultTheme->getMediaQuery()); + $this->assertEquals('', $this->theme->getMediaQuery()); } public function testGetCustomCss() { - $this->assertEquals('', $this->defaultTheme->getCustomCss()); + $this->assertEquals('', $this->theme->getCustomCss()); } /** @@ -151,7 +148,7 @@ class DefaultThemeTest extends TestCase { public function testThemindDisabledFallbackCss() { // Generate variables $variables = ''; - foreach ($this->defaultTheme->getCSSVariables() as $variable => $value) { + foreach ($this->theme->getCSSVariables() as $variable => $value) { $variables .= " $variable: $value;" . PHP_EOL; }; -- 2.39.5