Use brand color for background only and keep accessible color as color primary

Pytal · blizzz · commit c6f98e21eef7 · 2022-10-03T19:15:07.000Z
Signed-off-by: Christopher Ng &lt;chrng8@gmail.com&gt;
diff --git a/apps/theming/css/default.css b/apps/theming/css/default.css
@@ -1,5 +1,6 @@
 :root {
   --color-main-background: #ffffff;
+  --color-main-background-not-plain: #0082c9;
   --color-main-background-rgb: 255,255,255;
   --color-main-background-translucent: rgba(var(--color-main-background-rgb), .97);
   --color-main-background-blur: rgba(var(--color-main-background-rgb), .8);
diff --git a/apps/theming/css/settings-admin.css b/apps/theming/css/settings-admin.css
diff --git a/apps/theming/css/settings-admin.scss b/apps/theming/css/settings-admin.scss
@@ -100,7 +100,7 @@
         margin-top: 10px;
         margin-bottom: 20px;
         cursor: pointer;
-        background-color: var(--color-primary);
+        background-color: var(--color-main-background-not-plain, var(--color-primary));
         background-image: var(--image-background, var(--image-background-plain, url('../../../core/img/app-background.jpg'), linear-gradient(40deg, #0082c9 0%, #30b6ff 100%)));
 
         #theming-preview-logo {
@@ -145,4 +145,4 @@
     svg, img {
         transition: 500ms filter linear;
     }
-}
+}
diff --git a/apps/theming/lib/Service/BackgroundService.php b/apps/theming/lib/Service/BackgroundService.php
@@ -44,6 +44,8 @@
 class BackgroundService {
 	// true when the background is bright and need dark icons
 	public const THEMING_MODE_DARK = 'dark';
+	public const DEFAULT_COLOR = '#0082c9';
+	public const DEFAULT_ACCESSIBLE_COLOR = '#00639a';
 
 	public const SHIPPED_BACKGROUNDS = [
 		'anatoly-mikhaltsov-butterfly-wing-scale.jpg' => [
@@ -90,8 +92,7 @@ class BackgroundService {
 		'kamil-porembinski-clouds.jpg' => [
 			'attribution' => 'Clouds (Kamil Porembiński, CC BY-SA)',
 			'attribution_url' => 'https://www.flickr.com/photos/paszczak000/8715851521/',
-			// Originally #0082c9 but adjusted for accessibility
-			'primary_color' => '#00639a',
+			'primary_color' => self::DEFAULT_COLOR,
 		],
 		'bernard-spragg-new-zealand-fern.jpg' => [
 			'attribution' => 'New zealand fern (Bernard Spragg, CC0)',
diff --git a/apps/theming/lib/Themes/DefaultTheme.php b/apps/theming/lib/Themes/DefaultTheme.php
@@ -65,7 +65,9 @@ public function __construct(Util $util,
 		$this->config = $config;
 		$this->l = $l;
 
-		$this->primaryColor = $this->themingDefaults->getColorPrimary();
+		$initialPrimaryColor = $this->themingDefaults->getColorPrimary();
+		// Override default color if set to improve accessibility
+		$this->primaryColor = $initialPrimaryColor === BackgroundService::DEFAULT_COLOR ? BackgroundService::DEFAULT_ACCESSIBLE_COLOR : $initialPrimaryColor;
 	}
 
 	public function getId(): string {
@@ -105,6 +107,7 @@ public function getCSSVariables(): array {
 
 		$variables = [
 			'--color-main-background' => $colorMainBackground,
+			'--color-main-background-not-plain' => $this->themingDefaults->getColorPrimary(),
 			'--color-main-background-rgb' => $colorMainBackgroundRGB,
 			'--color-main-background-translucent' => 'rgba(var(--color-main-background-rgb), .97)',
 			'--color-main-background-blur' => 'rgba(var(--color-main-background-rgb), .8)',
@@ -235,7 +238,7 @@ public function getCSSVariables(): array {
 				$variables['--image-main-background'] = "url('" . $this->urlGenerator->linkTo(Application::APP_ID, "/img/background/$themingBackground") . "')";
 			} elseif (substr($themingBackground, 0, 1) === '#') {
 				unset($variables['--image-main-background']);
-				$variables['--color-main-background-plain'] = $this->primaryColor;
+				$variables['--color-main-background-plain'] = $this->themingDefaults->getColorPrimary();
 			}
 		}
 
diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php
@@ -223,17 +223,17 @@ public function getColorPrimary() {
 
 		if ($color === '' && !empty($user)) {
 			$themingBackground = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background', 'default');
-			if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'])) {
+			if ($themingBackground === 'default') {
 				$this->increaseCacheBuster();
-				return BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'];
-			} else if ($themingBackground === 'default') {
+				return BackgroundService::DEFAULT_COLOR;
+			} else if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'])) {
 				$this->increaseCacheBuster();
-				return BackgroundService::SHIPPED_BACKGROUNDS['kamil-porembinski-clouds.jpg']['primary_color'];
+				return BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'];
 			}
 		}
 
 		if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) {
-			return BackgroundService::SHIPPED_BACKGROUNDS['kamil-porembinski-clouds.jpg']['primary_color'];
+			return BackgroundService::DEFAULT_COLOR;
 		}
 
 		return $color;
diff --git a/apps/theming/src/components/BackgroundSettings.vue b/apps/theming/src/components/BackgroundSettings.vue
@@ -171,7 +171,7 @@ export default {
 		}
 
 		&.color {
-			background-color: var(--color-primary);
+			background-color: var(--color-main-background-not-plain, var(--color-primary));
 			color: var(--color-primary-text);
 		}
 
diff --git a/core/css/guest.css b/core/css/guest.css
@@ -23,7 +23,7 @@ body {
 	font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen-Sans, Ubuntu, Cantarell, 'Helvetica Neue', Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
 	color: var(--color-text);
 	text-align: center;
-	background-color: var(--color-primary);
+	background-color: var(--color-main-background-not-plain, var(--color-primary));
 	background-image: var(--image-background, var(--image-background-plain, url('../../../core/img/app-background.jpg'), linear-gradient(40deg, #0082c9 0%, #30b6ff 100%)));
     background-attachment: fixed;
 	min-height: 100%; /* fix sticky footer */
diff --git a/dist/theming-theming-settings.js b/dist/theming-theming-settings.js
diff --git a/dist/theming-theming-settings.js.map b/dist/theming-theming-settings.js.map
diff --git a/tests/acceptance/features/app-theming.feature b/tests/acceptance/features/app-theming.feature
@@ -9,10 +9,12 @@ Feature: app-theming
     # The "eventually" part is not really needed here, as the colour is not
     # being animated at this point, but there is no need to create a specific
     # step just for this.
-    And I see that the primary color is eventually "#0082C9"
+    And I see that the primary color is eventually "#00639a"
+    And I see that the non-plain background color variable is eventually "#0082c9"
     When I set the "Color" parameter in the Theming app to "#C9C9C9"
     Then I see that the parameters in the Theming app are eventually saved
     And I see that the primary color is eventually "#C9C9C9"
+    And I see that the non-plain background color variable is eventually "#C9C9C9"
 
   Scenario: resetting the color updates the primary color
     Given I am logged in as the admin
@@ -22,6 +24,8 @@ Feature: app-theming
     And I set the "Color" parameter in the Theming app to "#C9C9C9"
     And I see that the parameters in the Theming app are eventually saved
     And I see that the primary color is eventually "#C9C9C9"
+    And I see that the non-plain background color variable is eventually "#C9C9C9"
     When I reset the "Color" parameter in the Theming app to its default value
     Then I see that the parameters in the Theming app are eventually saved
-    And I see that the primary color is eventually "#0082C9"
+    And I see that the primary color is eventually "#00639a"
+    And I see that the non-plain background color variable is eventually "#0082c9"
diff --git a/tests/acceptance/features/bootstrap/ThemingAppContext.php b/tests/acceptance/features/bootstrap/ThemingAppContext.php
@@ -141,6 +141,23 @@ public function iSeeThatThePrimaryColorIsEventually($color) {
 		}
 	}
 
+	/**
+	 * @Then I see that the non-plain background color variable is eventually :color
+	 */
+	public function iSeeThatTheNonPlainBackgroundColorVariableIsEventually($color) {
+		$colorVariableMatchesCallback = function () use ($color) {
+			$colorVariable = $this->actor->getSession()->evaluateScript("return getComputedStyle(document.documentElement).getPropertyValue('--color-main-background-not-plain').trim();");
+			$colorVariable = $this->getRGBArray($colorVariable);
+			$color = $this->getRGBArray($color);
+
+			return $colorVariable == $color;
+		};
+
+		if (!Utils::waitFor($colorVariableMatchesCallback, $timeout = 10 * $this->actor->getFindTimeoutMultiplier(), $timeoutStep = 1)) {
+			Assert::fail("The non-plain background color variable is not $color yet after $timeout seconds");
+		}
+	}
+
 	/**
 	 * @Then I see that the parameters in the Theming app are eventually saved
 	 */

Original file line number	Diff line number	Diff line change
`@@ -171,7 +171,7 @@ export default {`
`171`	`171`	`}`
`172`	`172`
`173`	`173`	`&.color {`
`174`		`- background-color: var(--color-primary);`
	`174`	`+ background-color: var(--color-main-background-not-plain, var(--color-primary));`
`175`	`175`	`color: var(--color-primary-text);`
`176`	`176`	`}`
`177`	`177`