Merge pull request #34437 from nextcloud/feat/theming-default-system-value

Use default system primary

apps/theming/css/default.css

@@ -57,6 +57,7 @@
   --background-invert-if-bright: invert(100%);
   --image-main-background: url('/core/img/app-background.jpg');
   --color-primary: #00639a;
+  --color-primary-default: #0082c9;
   --color-primary-text: #ffffff;
   --color-primary-hover: #3282ae;
   --color-primary-light: #e5eff4;

apps/theming/css/settings-admin.css

@@ -89,7 +89,7 @@
   margin-top: 10px;
   margin-bottom: 20px;
   cursor: pointer;
-  background-color: var(--color-main-background-not-plain, var(--color-primary));
+  background-color: var(--color-primary-default);
   background-image: var(--image-background, var(--image-background-plain, url("../../../core/img/app-background.jpg"), linear-gradient(40deg, #0082c9 0%, #30b6ff 100%)));
 }
 #theming #theming-preview #theming-preview-logo {

apps/theming/css/settings-admin.scss

@@ -100,7 +100,7 @@
         margin-top: 10px;
         margin-bottom: 20px;
         cursor: pointer;
-        background-color: var(--color-main-background-not-plain, var(--color-primary));
+        background-color: var(--color-primary-default);
         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 {

apps/theming/lib/Service/JSDataService.php

@@ -55,6 +55,7 @@ class JSDataService implements \JsonSerializable {
 			'url' => $this->themingDefaults->getBaseUrl(),
 			'slogan' => $this->themingDefaults->getSlogan(),
 			'color' => $this->themingDefaults->getColorPrimary(),
+			'defaultColor' => $this->themingDefaults->getDefaultColorPrimary(),
 			'imprintUrl' => $this->themingDefaults->getImprintUrl(),
 			'privacyUrl' => $this->themingDefaults->getPrivacyUrl(),
 			'inverted' => $this->util->invertTextColor($this->themingDefaults->getColorPrimary()),

apps/theming/lib/Settings/Admin.php

@@ -75,7 +75,7 @@ class Admin implements IDelegatedSettings {
 			'name' => $this->themingDefaults->getEntity(),
 			'url' => $this->themingDefaults->getBaseUrl(),
 			'slogan' => $this->themingDefaults->getSlogan(),
-			'color' => $this->themingDefaults->getColorPrimary(),
+			'color' => $this->themingDefaults->getDefaultColorPrimary(),
 			'uploadLogoRoute' => $this->urlGenerator->linkToRoute('theming.Theming.uploadImage'),
 			'canThemeIcons' => $this->imageManager->shouldReplaceIcons(),
 			'iconDocs' => $this->urlGenerator->linkToDocs('admin-theming-icons'),

apps/theming/lib/Themes/CommonThemeTrait.php

@@ -42,6 +42,7 @@ trait CommonThemeTrait {
 		// primary related colours
 		return [
 			'--color-primary' => $this->primaryColor,
+			'--color-primary-default' => $this->defaultPrimaryColor,
 			'--color-primary-text' => $this->util->invertTextColor($this->primaryColor) ? '#000000' : '#ffffff',
 			'--color-primary-hover' => $this->util->mix($this->primaryColor, $colorMainBackground, 60),
 			'--color-primary-light' => $colorPrimaryLight,

apps/theming/lib/Themes/DefaultTheme.php

@@ -48,6 +48,7 @@ class DefaultTheme implements ITheme {
 	public IConfig $config;
 	public IL10N $l;
 
+	public string $defaultPrimaryColor;
 	public string $primaryColor;
 
 	public function __construct(Util $util,
@@ -65,9 +66,13 @@ class DefaultTheme implements ITheme {
 		$this->config = $config;
 		$this->l = $l;
 
-		$initialPrimaryColor = $this->themingDefaults->getColorPrimary();
-		// Override default color if set to improve accessibility
-		$this->primaryColor = $initialPrimaryColor === BackgroundService::DEFAULT_COLOR ? BackgroundService::DEFAULT_ACCESSIBLE_COLOR : $initialPrimaryColor;
+		$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary();
+		$this->primaryColor = $this->themingDefaults->getColorPrimary();
+
+		// Override default defaultPrimaryColor if set to improve accessibility
+		if ($this->primaryColor === BackgroundService::DEFAULT_COLOR) {
+			$this->primaryColor = BackgroundService::DEFAULT_ACCESSIBLE_COLOR;
+		}
 	}
 
 	public function getId(): string {

apps/theming/lib/ThemingDefaults.php

@@ -214,26 +214,50 @@ class ThemingDefaults extends \OC_Defaults {
 
 	/**
 	 * Color that is used for the header as well as for mail headers
-	 *
-	 * @return string
 	 */
-	public function getColorPrimary() {
+	public function getColorPrimary(): string {
 		$user = $this->userSession->getUser();
-		$color = $this->config->getAppValue(Application::APP_ID, 'color', '');
 
-		if ($color === '' && !empty($user)) {
-			$themingBackground = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background', 'default');
-			if ($themingBackground === 'default') {
+		// admin-defined primary color
+		$defaultColor = $this->getDefaultColorPrimary();
+		
+		// user-defined primary color
+		$themingBackground = '';
+		if (!empty($user)) {
+			$themingBackground = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background', '');
+			// If the user selected the default background
+			if ($themingBackground === '') {
 				return BackgroundService::DEFAULT_COLOR;
-			} else if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'])) {
+			}
+
+			// If the user selected a specific colour
+			if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $themingBackground)) {
+				return $themingBackground;
+			}
+
+			// if the user-selected background is a background reference
+			if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'])) {
 				return BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground]['primary_color'];
 			}
 		}
 
-		if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) {
+		// If the default color is not valid, return the default background one
+		if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) {
 			return BackgroundService::DEFAULT_COLOR;
 		}
 
+		// Finally, return the system global primary color
+		return $defaultColor;
+	}
+
+	/**
+	 * Return the default color primary
+	 */
+	public function getDefaultColorPrimary(): string {
+		$color = $this->config->getAppValue(Application::APP_ID, 'color');
+		if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) {
+			$color = '#0082c9';
+		}
 		return $color;
 	}
 

apps/theming/src/UserThemes.vue

@@ -156,16 +156,16 @@ export default {
 		},
 	},
 
-	mounted() {
-		this.updateGlobalStyles()
-	},
-
 	watch: {
 		shortcutsDisabled(newState) {
 			this.changeShortcutsDisabled(newState)
 		},
 	},
 
+	mounted() {
+		this.updateGlobalStyles()
+	},
+
 	methods: {
 		updateBackground(data) {
 			this.background = (data.type === 'custom' || data.type === 'default') ? data.type : data.value

apps/theming/src/components/BackgroundSettings.vue

@@ -25,42 +25,67 @@
 
 <template>
 	<div class="background-selector">
+		<!-- Custom background -->
 		<button class="background filepicker"
 			:class="{ active: background === 'custom' }"
 			tabindex="0"
 			@click="pickFile">
 			{{ t('theming', 'Pick from Files') }}
 		</button>
+
+		<!-- Default background -->
 		<button class="background default"
 			tabindex="0"
 			:class="{ 'icon-loading': loading === 'default', active: background === 'default' }"
 			@click="setDefault">
 			{{ t('theming', 'Default image') }}
 		</button>
+
+		<!-- Custom color picker -->
+		<NcColorPicker v-model="Theming.color" @input="debouncePickColor">
+			<button class="background color"
+				:class="{ active: background === Theming.color}"
+				tabindex="0"
+				:data-color="Theming.color"
+				:data-color-bright="invertTextColor(Theming.color)"
+				:style="{ backgroundColor: Theming.color, color: invertTextColor(Theming.color) ? '#000000' : '#ffffff'}">
+				{{ t('theming', 'Custom color') }}
+			</button>
+		</NcColorPicker>
+
+		<!-- Default admin primary color -->
 		<button class="background color"
-			:class="{ active: background.startsWith('#') }"
+			:class="{ active: background === Theming.defaultColor }"
 			tabindex="0"
-			@click="pickColor">
+			:data-color="Theming.defaultColor"
+			:data-color-bright="invertTextColor(Theming.defaultColor)"
+			:style="{ color: invertTextColor(Theming.defaultColor) ? '#000000' : '#ffffff'}"
+			@click="debouncePickColor">
 			{{ t('theming', 'Plain background') }}
 		</button>
+
+		<!-- Background set selection -->
 		<button v-for="shippedBackground in shippedBackgrounds"
 			:key="shippedBackground.name"
 			v-tooltip="shippedBackground.details.attribution"
 			:class="{ 'icon-loading': loading === shippedBackground.name, active: background === shippedBackground.name }"
 			tabindex="0"
 			class="background"
+			:data-color-bright="shippedBackground.details.theming === 'dark'"
 			:style="{ 'background-image': 'url(' + shippedBackground.preview + ')' }"
 			@click="setShipped(shippedBackground.name)" />
 	</div>
 </template>
 
 <script>
-import axios from '@nextcloud/axios'
-import Tooltip from '@nextcloud/vue/dist/Directives/Tooltip'
 import { generateUrl } from '@nextcloud/router'
-import { loadState } from '@nextcloud/initial-state'
 import { getBackgroundUrl } from '../helpers/getBackgroundUrl.js'
+import { loadState } from '@nextcloud/initial-state'
 import { prefixWithBaseUrl } from '../helpers/prefixWithBaseUrl.js'
+import axios from '@nextcloud/axios'
+import debounce from 'debounce'
+import NcColorPicker from '@nextcloud/vue/dist/Components/NcColorPicker'
+import Tooltip from '@nextcloud/vue/dist/Directives/Tooltip'
 
 const shippedBackgroundList = loadState('theming', 'shippedBackgrounds')
 
@@ -69,6 +94,11 @@ export default {
 	directives: {
 		Tooltip,
 	},
+
+	components: {
+		NcColorPicker,
+	},
+
 	props: {
 		background: {
 			type: String,
@@ -79,12 +109,15 @@ export default {
 			default: '',
 		},
 	},
+
 	data() {
 		return {
 			backgroundImage: generateUrl('/apps/theming/background') + '?v=' + Date.now(),
 			loading: false,
+			Theming: loadState('theming', 'data', {}),
 		}
 	},
+
 	computed: {
 		shippedBackgrounds() {
 			return Object.keys(shippedBackgroundList).map(fileName => {
@@ -97,7 +130,39 @@ export default {
 			})
 		},
 	},
+
 	methods: {
+		/**
+		 * Do we need to invert the text if color is too bright?
+		 *
+		 * @param {string} color the hex color
+		 */
+		invertTextColor(color) {
+			return this.calculateLuma(color) > 0.6
+		},
+
+		/**
+		 * Calculate luminance of provided hex color
+		 *
+		 * @param {string} color the hex color
+		 */
+		calculateLuma(color) {
+			const [red, green, blue] = this.hexToRGB(color)
+			return (0.2126 * red + 0.7152 * green + 0.0722 * blue) / 255
+		},
+
+		/**
+		 * Convert hex color to RGB
+		 *
+		 * @param {string} hex the hex color
+		 */
+		hexToRGB(hex) {
+			const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex)
+			return result
+				? [parseInt(result[1], 16), parseInt(result[2], 16), parseInt(result[3], 16)]
+				: null
+		},
+
 		async update(data) {
 			const background = data.type === 'custom' || data.type === 'default' ? data.type : data.value
 			this.backgroundImage = getBackgroundUrl(background, data.version, this.themingDefaultBackground)
@@ -113,27 +178,35 @@ export default {
 			}
 			image.src = this.backgroundImage
 		},
+
 		async setDefault() {
 			this.loading = 'default'
 			const result = await axios.post(generateUrl('/apps/theming/background/default'))
 			this.update(result.data)
 		},
+
 		async setShipped(shipped) {
 			this.loading = shipped
 			const result = await axios.post(generateUrl('/apps/theming/background/shipped'), { value: shipped })
 			this.update(result.data)
 		},
+
 		async setFile(path) {
 			this.loading = 'custom'
 			const result = await axios.post(generateUrl('/apps/theming/background/custom'), { value: path })
 			this.update(result.data)
 		},
-		async pickColor() {
+
+		debouncePickColor: debounce(function() {
+			this.pickColor(...arguments)
+		}, 200),
+		async pickColor(event) {
 			this.loading = 'color'
-			const color = OCA && OCA.Theming ? OCA.Theming.color : '#0082c9'
+			const color = event?.target?.dataset?.color || this.Theming?.color || '#0082c9'
 			const result = await axios.post(generateUrl('/apps/theming/background/color'), { value: color })
 			this.update(result.data)
 		},
+
 		pickFile() {
 			window.OC.dialogs.filepicker(t('theming', 'Insert from {productName}', { productName: OC.theme.name }), (path, type) => {
 				if (type === OC.dialogs.FILEPICKER_TYPE_CHOOSE) {
@@ -171,7 +244,7 @@ export default {
 		}
 
 		&.color {
-			background-color: var(--color-main-background-not-plain, var(--color-primary));
+			background-color: var(--color-primary-default);
 			color: var(--color-primary-text);
 		}
 
@@ -181,14 +254,20 @@ export default {
 			border: 2px solid var(--color-primary);
 		}
 
-		&.active:not(.icon-loading):after {
-			background-image: var(--icon-checkmark-white);
-			background-repeat: no-repeat;
-			background-position: center;
-			background-size: 44px;
-			content: '';
-			display: block;
-			height: 100%;
+		&.active:not(.icon-loading) {
+			&:after {
+				background-image: var(--icon-checkmark-white);
+				background-repeat: no-repeat;
+				background-position: center;
+				background-size: 44px;
+				content: '';
+				display: block;
+				height: 100%;
+			}
+
+			&[data-color-bright]:after {
+				background-image: var(--icon-checkmark-dark);
+			}
 		}
 	}
 }

apps/theming/tests/Service/ThemesServiceTest.php

@@ -68,6 +68,10 @@ class ThemesServiceTest extends TestCase {
 			->method('getColorPrimary')
 			->willReturn('#0082c9');
 
+		$this->themingDefaults->expects($this->any())
+			->method('getDefaultColorPrimary')
+			->willReturn('#0082c9');
+
 		$this->initThemes();
 
 		$this->themesService = new ThemesService(

apps/theming/tests/Settings/AdminTest.php

@@ -97,7 +97,7 @@ class AdminTest extends TestCase {
 			->willReturn('MySlogan');
 		$this->themingDefaults
 			->expects($this->once())
-			->method('getColorPrimary')
+			->method('getDefaultColorPrimary')
 			->willReturn('#fff');
 		$this->urlGenerator
 			->expects($this->once())
@@ -156,7 +156,7 @@ class AdminTest extends TestCase {
 			->willReturn('MySlogan');
 		$this->themingDefaults
 			->expects($this->once())
-			->method('getColorPrimary')
+			->method('getDefaultColorPrimary')
 			->willReturn('#fff');
 		$this->urlGenerator
 			->expects($this->once())

apps/theming/tests/Settings/PersonalTest.php

@@ -138,6 +138,10 @@ class PersonalTest extends TestCase {
 		$themingDefaults->expects($this->any())
 			->method('getColorPrimary')
 			->willReturn('#0082c9');
+		
+		$themingDefaults->expects($this->any())
+			->method('getDefaultColorPrimary')
+			->willReturn('#0082c9');
 
 		$this->themes = [
 			'default' => new DefaultTheme(

apps/theming/tests/Themes/DefaultThemeTest.php

@@ -70,6 +70,11 @@ class DefaultThemeTest extends TestCase {
 			->method('getColorPrimary')
 			->willReturn('#0082c9');
 
+		$this->themingDefaults
+			->expects($this->any())
+			->method('getDefaultColorPrimary')
+			->willReturn('#0082c9');
+
 		$this->l10n
 			->expects($this->any())
 			->method('t')

apps/theming/tests/Themes/DyslexiaFontTest.php

@@ -84,6 +84,11 @@ class DyslexiaFontTest extends TestCase {
 			->method('getColorPrimary')
 			->willReturn('#0082c9');
 
+		$this->themingDefaults
+			->expects($this->any())
+			->method('getDefaultColorPrimary')
+			->willReturn('#0082c9');
+
 		$this->l10n
 			->expects($this->any())
 			->method('t')

apps/theming/tests/ThemingDefaultsTest.php

@@ -35,6 +35,7 @@
 namespace OCA\Theming\Tests;
 
 use OCA\Theming\ImageManager;
+use OCA\Theming\Service\BackgroundService;
 use OCA\Theming\ThemingDefaults;
 use OCA\Theming\Util;
 use OCP\App\IAppManager;
@@ -46,6 +47,7 @@ use OCP\IConfig;
 use OCP\IL10N;
 use OCP\INavigationManager;
 use OCP\IURLGenerator;
+use OCP\IUser;
 use OCP\IUserSession;
 use Test\TestCase;
 
@@ -420,7 +422,7 @@ class ThemingDefaultsTest extends TestCase {
 		$this->assertEquals('<a href="url" target="_blank" rel="noreferrer noopener" class="entity-name">Name</a> – Slogan', $this->template->getShortFooter());
 	}
 
-	public function testgetColorPrimaryWithDefault() {
+	public function testGetColorPrimaryWithDefault() {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
@@ -440,6 +442,74 @@ class ThemingDefaultsTest extends TestCase {
 		$this->assertEquals('#fff', $this->template->getColorPrimary());
 	}
 
+	public function testGetColorPrimaryWithDefaultBackground() {
+		$user = $this->createMock(IUser::class);
+		$this->userSession->expects($this->any())
+			->method('getUser')
+			->willReturn($user);
+		$user->expects($this->any())
+			->method('getUID')
+			->willReturn('user');
+
+		$this->assertEquals(BackgroundService::DEFAULT_COLOR, $this->template->getColorPrimary());
+	}
+
+	public function testGetColorPrimaryWithCustomBackground() {
+		$backgroundIndex = 2;
+		$background = array_values(BackgroundService::SHIPPED_BACKGROUNDS)[$backgroundIndex];
+		$user = $this->createMock(IUser::class);
+		$this->userSession->expects($this->any())
+			->method('getUser')
+			->willReturn($user);
+		$user->expects($this->any())
+			->method('getUID')
+			->willReturn('user');
+
+		$this->config
+			->expects($this->once())
+			->method('getUserValue')
+			->with('user', 'theming', 'background', '')
+			->willReturn(array_keys(BackgroundService::SHIPPED_BACKGROUNDS)[$backgroundIndex]);
+
+		$this->assertEquals($background['primary_color'], $this->template->getColorPrimary());
+	}
+
+	public function testGetColorPrimaryWithCustomBackgroundColor() {
+		$user = $this->createMock(IUser::class);
+		$this->userSession->expects($this->any())
+			->method('getUser')
+			->willReturn($user);
+		$user->expects($this->any())
+			->method('getUID')
+			->willReturn('user');
+
+		$this->config
+			->expects($this->once())
+			->method('getUserValue')
+			->with('user', 'theming', 'background', '')
+			->willReturn('#fff');
+
+		$this->assertEquals('#fff', $this->template->getColorPrimary());
+	}
+
+	public function testGetColorPrimaryWithInvalidCustomBackgroundColor() {
+		$user = $this->createMock(IUser::class);
+		$this->userSession->expects($this->any())
+			->method('getUser')
+			->willReturn($user);
+		$user->expects($this->any())
+			->method('getUID')
+			->willReturn('user');
+
+		$this->config
+			->expects($this->once())
+			->method('getUserValue')
+			->with('user', 'theming', 'background', '')
+			->willReturn('nextcloud');
+
+		$this->assertEquals($this->template->getDefaultColorPrimary(), $this->template->getColorPrimary());
+	}
+
 	public function testSet() {
 		$this->config
 			->expects($this->exactly(2))

dist/core-common.js.LICENSE.txt

@@ -380,6 +380,8 @@
 
 /*! For license information please see NcCheckboxRadioSwitch.js.LICENSE.txt */
 
+/*! For license information please see NcColorPicker.js.LICENSE.txt */
+
 /*! For license information please see NcDatetimePicker.js.LICENSE.txt */
 
 /*! For license information please see NcHighlight.js.LICENSE.txt */

tests/acceptance/features/app-theming.feature

@@ -13,7 +13,7 @@ Feature: app-theming
     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 primary color is eventually "#00639a"
     And I see that the non-plain background color variable is eventually "#C9C9C9"
 
   Scenario: resetting the color updates the primary color
@@ -23,7 +23,7 @@ Feature: app-theming
     And I see that the color selector in the Theming app has loaded
     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 primary color is eventually "#00639a"
     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

tests/acceptance/features/bootstrap/ThemingAppContext.php

@@ -146,7 +146,7 @@ class ThemingAppContext implements Context, ActorAwareInterface {
 	 */
 	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->actor->getSession()->evaluateScript("return getComputedStyle(document.documentElement).getPropertyValue('--color-primary-default').trim();");
 			$colorVariable = $this->getRGBArray($colorVariable);
 			$color = $this->getRGBArray($color);