Merge pull request #38159 from nextcloud/enh/noid/fix-themed-icons

fix too dark or bright primary element color

3 files changed, 39 insertions, 24 deletions

diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php
index f5321dada67..789da394567 100644
--- a/apps/theming/lib/Util.php
+++ b/apps/theming/lib/Util.php
@@ -78,20 +78,20 @@ class Util {
 	 * get color for on-page elements:
 	 * theme color by default, grey if theme color is to bright
 	 * @param string $color
-	 * @param bool $brightBackground
+	 * @param ?bool $brightBackground
 	 * @return string
 	 */
-	public function elementColor($color, bool $brightBackground = true) {
+	public function elementColor($color, ?bool $brightBackground = null) {
 		$luminance = $this->calculateLuminance($color);
 
-		if ($brightBackground && $luminance > 0.8) {
-			// If the color is too bright in bright mode, we fall back to a darker gray
-			return '#aaaaaa';
+		if ($brightBackground !== false && $luminance > 0.8) {
+			// If the color is too bright in bright mode, we fall back to a darkened color
+			return $this->darken($color, 30);
 		}
 
-		if (!$brightBackground && $luminance < 0.2) {
-			// If the color is too dark in dark mode, we fall back to a brighter gray
-			return '#8c8c8c';
+		if ($brightBackground !== true && $luminance < 0.2) {
+			// If the color is too dark in dark mode, we fall back to a brightened color
+			return $this->lighten($color, 30);
 		}
 
 		return $color;
diff --git a/apps/theming/tests/CapabilitiesTest.php b/apps/theming/tests/CapabilitiesTest.php
index 1c968383d53..1a2d01d69d1 100644
--- a/apps/theming/tests/CapabilitiesTest.php
+++ b/apps/theming/tests/CapabilitiesTest.php
@@ -77,8 +77,8 @@ class CapabilitiesTest extends TestCase {
 				'slogan' => 'slogan',
 				'color' => '#FFFFFF',
 				'color-text' => '#000000',
-				'color-element' => '#aaaaaa',
-				'color-element-bright' => '#aaaaaa',
+				'color-element' => '#b3b3b3',
+				'color-element-bright' => '#b3b3b3',
 				'color-element-dark' => '#FFFFFF',
 				'logo' => 'http://absolute/logo',
 				'background' => 'http://absolute/background',
@@ -109,9 +109,9 @@ class CapabilitiesTest extends TestCase {
 				'slogan' => 'slogan3',
 				'color' => '#000000',
 				'color-text' => '#ffffff',
-				'color-element' => '#000000',
-				'color-element-bright' => '#000000',
-				'color-element-dark' => '#8c8c8c',
+				'color-element' => '#4d4d4d',
+				'color-element-bright' => '#4d4d4d',
+				'color-element-dark' => '#4d4d4d',
 				'logo' => 'http://localhost/logo5',
 				'background' => '#000000',
 				'background-plain' => true,
@@ -125,9 +125,9 @@ class CapabilitiesTest extends TestCase {
 				'slogan' => 'slogan3',
 				'color' => '#000000',
 				'color-text' => '#ffffff',
-				'color-element' => '#000000',
-				'color-element-bright' => '#000000',
-				'color-element-dark' => '#8c8c8c',
+				'color-element' => '#4d4d4d',
+				'color-element-bright' => '#4d4d4d',
+				'color-element-dark' => '#4d4d4d',
 				'logo' => 'http://localhost/logo5',
 				'background' => '#000000',
 				'background-plain' => true,
@@ -178,7 +178,7 @@ class CapabilitiesTest extends TestCase {
 		$this->util->expects($this->exactly(3))
 			->method('elementColor')
 			->with($color)
-			->willReturnCallback(static function (string $color, bool $brightBackground = true) use ($util) {
+			->willReturnCallback(static function (string $color, ?bool $brightBackground = null) use ($util) {
 				return $util->elementColor($color, $brightBackground);
 			});
 
diff --git a/apps/theming/tests/UtilTest.php b/apps/theming/tests/UtilTest.php
index 789107d9fdf..137038acb98 100644
--- a/apps/theming/tests/UtilTest.php
+++ b/apps/theming/tests/UtilTest.php
@@ -105,19 +105,34 @@ class UtilTest extends TestCase {
 		$this->util->invertTextColor('');
 	}
 
-	public function testElementColorDefault() {
+	public function testElementColorDefaultBlack() {
 		$elementColor = $this->util->elementColor("#000000");
-		$this->assertEquals('#000000', $elementColor);
+		$this->assertEquals('#4d4d4d', $elementColor);
+	}
+
+	public function testElementColorDefaultWhite() {
+		$elementColor = $this->util->elementColor("#ffffff");
+		$this->assertEquals('#b3b3b3', $elementColor);
 	}
 
-	public function testElementColorOnDarkBackground() {
+	public function testElementColorBlackOnDarkBackground() {
 		$elementColor = $this->util->elementColor("#000000", false);
-		$this->assertEquals('#8c8c8c', $elementColor);
+		$this->assertEquals('#4d4d4d', $elementColor);
+	}
+
+	public function testElementColorBlackOnBrightBackground() {
+		$elementColor = $this->util->elementColor("#000000", true);
+		$this->assertEquals('#000000', $elementColor);
+	}
+
+	public function testElementColorWhiteOnBrightBackground() {
+		$elementColor = $this->util->elementColor('#ffffff', true);
+		$this->assertEquals('#b3b3b3', $elementColor);
 	}
 
-	public function testElementColorOnBrightBackground() {
-		$elementColor = $this->util->elementColor('#ffffff');
-		$this->assertEquals('#aaaaaa', $elementColor);
+	public function testElementColorWhiteOnDarkBackground() {
+		$elementColor = $this->util->elementColor('#ffffff', false);
+		$this->assertEquals('#ffffff', $elementColor);
 	}
 
 	public function testGenerateRadioButtonWhite() {