Invert header if primary is bright and background disabled

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>

apps/theming/lib/Controller/UserThemeController.php

@@ -50,16 +50,14 @@ use OCP\PreConditionNotMetException;
 
 class UserThemeController extends OCSController {
 
-	protected string $userId;
+	protected ?string $userId = null;
+	
 	private IConfig $config;
 	private IUserSession $userSession;
 	private ThemesService $themesService;
 	private ThemingDefaults $themingDefaults;
 	private BackgroundService $backgroundService;
 
-	/**
-	 * Config constructor.
-	 */
 	public function __construct(string $appName,
 								IRequest $request,
 								IConfig $config,
@@ -73,7 +71,11 @@ class UserThemeController extends OCSController {
 		$this->themesService = $themesService;
 		$this->themingDefaults = $themingDefaults;
 		$this->backgroundService = $backgroundService;
-		$this->userId = $userSession->getUser()->getUID();
+
+		$user = $userSession->getUser();
+		if ($user !== null) {
+			$this->userId = $user->getUID();
+		}
 	}
 
 	/**

apps/theming/lib/Themes/CommonThemeTrait.php

@@ -24,10 +24,10 @@ declare(strict_types=1);
  */
 namespace OCA\Theming\Themes;
 
-use OCA\Theming\AppInfo\Application;
+use OCA\Theming\Util;
 use OCA\Theming\ImageManager;
+use OCA\Theming\AppInfo\Application;
 use OCA\Theming\Service\BackgroundService;
-use OCA\Theming\Util;
 
 trait CommonThemeTrait {
 	public Util $util;
@@ -82,9 +82,9 @@ trait CommonThemeTrait {
 	 * Generate admin theming background-related variables
 	 */
 	protected function generateGlobalBackgroundVariables(): array {
-		$user = $this->userSession->getUser();
 		$backgroundDeleted = $this->config->getAppValue(Application::APP_ID, 'backgroundMime', '') === 'backgroundColor';
 		$hasCustomLogoHeader = $this->util->isLogoThemed();
+		$isDefaultPrimaryBright = $this->util->invertTextColor($this->defaultPrimaryColor);
 
 		$variables = [];
 
@@ -95,8 +95,10 @@ trait CommonThemeTrait {
 		// If primary as background has been request or if we have a custom primary colour
 		// let's not define the background image
 		if ($backgroundDeleted) {
-			$variables['--color-background-plain'] = $this->themingDefaults->getColorPrimary();
+			$variables['--color-background-plain'] = $this->defaultPrimaryColor;
 			$variables['--image-background-plain'] = 'yes';
+			// If no background image is set, we need to check against the shown primary colour
+			$variables['--background-image-invert-if-bright'] = $isDefaultPrimaryBright ? 'invert(100%)' : 'no';
 		}
 
 		// Register image variables only if custom-defined
@@ -125,12 +127,15 @@ trait CommonThemeTrait {
 			&& $this->appManager->isEnabledForUser(Application::APP_ID)) {
 			$backgroundImage = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_image', BackgroundService::BACKGROUND_DEFAULT);
 			$currentVersion = (int)$this->config->getUserValue($user->getUID(), Application::APP_ID, 'userCacheBuster', '0');
+			$isPrimaryBright = $this->util->invertTextColor($this->primaryColor);
 
 			// The user removed the background
 			if ($backgroundImage === BackgroundService::BACKGROUND_DISABLED) {
 				return [
 					'--image-background' => 'no',
-					'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
+					'--color-background-plain' => $this->primaryColor,
+					// If no background image is set, we need to check against the shown primary colour
+					'--background-image-invert-if-bright' => $isPrimaryBright ? 'invert(100%)' : 'no',
 				];
 			}
 

cypress/e2e/theming/admin-settings.cy.ts

@@ -32,7 +32,7 @@ const defaultBackground = 'kamil-porembinski-clouds.jpg'
 describe('Admin theming settings', function() {
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
@@ -53,7 +53,7 @@ describe('Change the primary colour and reset it', function() {
 	let selectedColor = ''
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
@@ -79,7 +79,7 @@ describe('Change the primary colour and reset it', function() {
 	})
 
 	it('Undo theming settings', function() {
-		cy.resetTheming()
+		cy.resetAdminTheming()
 	})
 
 	it('Screenshot the login page', function() {
@@ -92,7 +92,7 @@ describe('Change the primary colour and reset it', function() {
 describe('Remove the default background and restore it', function() {
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
@@ -122,7 +122,7 @@ describe('Remove the default background and restore it', function() {
 	})
 
 	it('Undo theming settings', function() {
-		cy.resetTheming()
+		cy.resetAdminTheming()
 	})
 
 	it('Screenshot the login page', function() {
@@ -132,6 +132,49 @@ describe('Remove the default background and restore it', function() {
 	})
 })
 
+describe.only('Remove the default background with a bright color', function() {
+	before(function() {
+		// Just in case previous test failed
+		cy.resetAdminTheming()
+		cy.resetUserTheming(admin)
+		cy.login(admin)
+	})
+
+	it('See the admin theming section', function() {
+		cy.visit('/settings/admin/theming')
+		cy.get('[data-admin-theming-settings]').scrollIntoView().should('be.visible')
+	})
+
+	it('Remove the default background', function() {
+		cy.intercept('*/apps/theming/ajax/updateStylesheet').as('removeBackground')
+
+		cy.get('[data-admin-theming-setting-file-remove]').click()
+
+		cy.wait('@removeBackground')
+	})
+
+	it('Change the primary colour', function() {
+		cy.intercept('*/apps/theming/ajax/updateStylesheet').as('setColor')
+
+		// Pick one of the bright color preset
+		cy.get('[data-admin-theming-setting-primary-color-picker]').click()
+		cy.get('.color-picker__simple-color-circle:eq(4)').click()
+
+		cy.wait('@setColor')
+		cy.waitUntil(() => validateBodyThemingCss('#ddcb55', ''))
+	})
+
+	it('See the header being inverted', function() {
+		cy.waitUntil(() => cy.window().then((win) => {
+			const firstEntry = win.document.querySelector('.app-menu-main li')
+			if (!firstEntry) {
+				return false
+			}
+			return getComputedStyle(firstEntry).filter === 'invert(1)'
+		}))
+	})
+})
+
 describe('Change the login fields then reset them', function() {
 	const name = 'ABCdef123'
 	const url = 'https://example.com'
@@ -139,7 +182,7 @@ describe('Change the login fields then reset them', function() {
 
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
@@ -196,7 +239,7 @@ describe('Change the login fields then reset them', function() {
 	})
 
 	it('Undo theming settings', function() {
-		cy.resetTheming()
+		cy.resetAdminTheming()
 	})
 
 	it('Check login screen changes', function() {
@@ -212,7 +255,7 @@ describe('Change the login fields then reset them', function() {
 describe('Disable user theming and enable it back', function() {
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
@@ -250,12 +293,12 @@ describe('User default option matches admin theming', function() {
 
 	before(function() {
 		// Just in case previous test failed
-		cy.resetTheming()
+		cy.resetAdminTheming()
 		cy.login(admin)
 	})
 
 	after(function() {
-		cy.resetTheming()
+		cy.resetAdminTheming()
 	})
 
 	it('See the admin theming section', function() {

cypress/e2e/theming/user-background.cy.ts

@@ -23,20 +23,8 @@ import type { User } from '@nextcloud/cypress'
 
 const defaultPrimary = '#006aa3'
 const defaultBackground = 'kamil-porembinski-clouds.jpg'
-import { colord } from 'colord'
 
-const validateThemingCss = function(expectedPrimary = '#0082c9', expectedBackground = 'kamil-porembinski-clouds.jpg', bright = false) {
-	return cy.window().then((win) => {
-		const backgroundColor = getComputedStyle(win.document.body).backgroundColor
-		const backgroundImage = getComputedStyle(win.document.body).backgroundImage
-		const invertIfBright = getComputedStyle(win.document.body).getPropertyValue('--background-image-invert-if-bright')
-
-		// Returning boolean for cy.waitUntil usage
-		return colord(backgroundColor).isEqual(expectedPrimary)
-			&& backgroundImage.includes(expectedBackground)
-			&& invertIfBright === (bright ? 'invert(100%)' : 'no')
-	})
-}
+import { pickRandomColor, validateBodyThemingCss } from './themingUtils'
 
 describe('User default background settings', function() {
 	before(function() {
@@ -61,7 +49,7 @@ describe('User default background settings', function() {
 	})
 })
 
-describe('User select shipped backgrounds', function() {
+describe('User select shipped backgrounds and remove background', function() {
 	before(function() {
 		cy.createRandomUser().then((user: User) => {
 			cy.login(user)
@@ -82,7 +70,7 @@ describe('User select shipped backgrounds', function() {
 
 		// Validate changed background and primary
 		cy.wait('@setBackground')
-		cy.waitUntil(() => validateThemingCss('#a53c17', background))
+		cy.waitUntil(() => validateBodyThemingCss('#a53c17', background))
 	})
 
 	it('Select a bright shipped background', function() {
@@ -94,7 +82,7 @@ describe('User select shipped backgrounds', function() {
 
 		// Validate changed background and primary
 		cy.wait('@setBackground')
-		cy.waitUntil(() => validateThemingCss('#56633d', background, true))
+		cy.waitUntil(() => validateBodyThemingCss('#56633d', background, true))
 	})
 
 	it('Remove background', function() {
@@ -105,7 +93,7 @@ describe('User select shipped backgrounds', function() {
 
 		// Validate clear background
 		cy.wait('@clearBackground')
-		cy.waitUntil(() => validateThemingCss('#56633d', ''))
+		cy.waitUntil(() => validateBodyThemingCss('#56633d', ''))
 	})
 })
 
@@ -124,10 +112,9 @@ describe('User select a custom color', function() {
 	it('Select a custom color', function() {
 		cy.intercept('*/apps/theming/background/color').as('setColor')
 
-		cy.get('[data-user-theming-background-color]').click()
-		cy.get('.color-picker__simple-color-circle:eq(3)').click()
+		pickRandomColor('[data-user-theming-background-color]')
 
-		// Validate clear background
+		// Validate custom colour change
 		cy.wait('@setColor')
 		cy.waitUntil(() => cy.window().then((win) => {
 			const primary = getComputedStyle(win.document.body).getPropertyValue('--color-primary')
@@ -136,6 +123,73 @@ describe('User select a custom color', function() {
 	})
 })
 
+describe('User select a bright custom color and remove background', function() {
+	before(function() {
+		cy.createRandomUser().then((user: User) => {
+			cy.login(user)
+		})
+	})
+
+	it('See the user background settings', function() {
+		cy.visit('/settings/user/theming')
+		cy.get('[data-user-theming-background-settings]').scrollIntoView().should('be.visible')
+	})
+
+	it('Remove background', function() {
+		cy.intercept('*/apps/theming/background/custom').as('clearBackground')
+
+		// Clear background
+		cy.get('[data-user-theming-background-clear]').click()
+
+		// Validate clear background
+		cy.wait('@clearBackground')
+		cy.waitUntil(() => validateBodyThemingCss(undefined, ''))
+	})
+
+	it('Select a custom color', function() {
+		cy.intercept('*/apps/theming/background/color').as('setColor')
+
+		// Pick one of the bright color preset
+		cy.get('[data-user-theming-background-color]').click()
+		cy.get('.color-picker__simple-color-circle:eq(4)').click()
+
+		// Validate custom colour change
+		cy.wait('@setColor')
+	})
+
+	it('See the header being inverted', function() {
+		cy.waitUntil(() => cy.window().then((win) => {
+			const firstEntry = win.document.querySelector('.app-menu-main li')
+			if (!firstEntry) {
+				return false
+			}
+			return getComputedStyle(firstEntry).filter === 'invert(1)'
+		}))
+	})
+
+	it('Select a shipped background', function() {
+		const background = 'anatoly-mikhaltsov-butterfly-wing-scale.jpg'
+		cy.intercept('*/apps/theming/background/shipped').as('setBackground')
+
+		// Select background
+		cy.get(`[data-user-theming-background-shipped="${background}"]`).click()
+
+		// Validate changed background and primary
+		cy.wait('@setBackground')
+		cy.waitUntil(() => validateBodyThemingCss('#a53c17', background))
+	})
+
+	it('See the header NOT being inverted', function() {
+		cy.waitUntil(() => cy.window().then((win) => {
+			const firstEntry = win.document.querySelector('.app-menu-main li')
+			if (!firstEntry) {
+				return false
+			}
+			return getComputedStyle(firstEntry).filter === 'none'
+		}))
+	})
+})
+
 describe('User select a custom background', function() {
 	const image = 'image.jpg'
 	before(function() {
@@ -160,7 +214,7 @@ describe('User select a custom background', function() {
 
 		// Wait for background to be set
 		cy.wait('@setBackground')
-		cy.waitUntil(() => validateThemingCss('#4c0c04', 'apps/theming/background?v='))
+		cy.waitUntil(() => validateBodyThemingCss('#4c0c04', 'apps/theming/background?v='))
 	})
 })
 
@@ -192,7 +246,7 @@ describe('User changes settings and reload the page', function() {
 
 		// Wait for background to be set
 		cy.wait('@setBackground')
-		cy.waitUntil(() => validateThemingCss(primaryFromImage, 'apps/theming/background?v='))
+		cy.waitUntil(() => validateBodyThemingCss(primaryFromImage, 'apps/theming/background?v='))
 	})
 
 	it('Select a custom color', function() {
@@ -211,6 +265,6 @@ describe('User changes settings and reload the page', function() {
 
 	it('Reload the page and validate persistent changes', function() {
 		cy.reload()
-		cy.waitUntil(() => validateThemingCss(selectedColor, 'apps/theming/background?v='))
+		cy.waitUntil(() => validateBodyThemingCss(selectedColor, 'apps/theming/background?v='))
 	})
 })

cypress/support/commands.ts

@@ -33,8 +33,24 @@ declare global {
 	// eslint-disable-next-line @typescript-eslint/no-namespace
 	namespace Cypress {
 		interface Chainable<Subject = any> {
-			uploadFile(user: User, fixture?: string, mimeType?: string, target ?: string): Cypress.Chainable<void>,
-			resetTheming(): Cypress.Chainable<void>,
+			/**
+			 * Upload a file from the fixtures folder to a given user storage.
+			 * **Warning**: Using this function will reset the previous session
+			 */
+			uploadFile(user: User, fixture?: string, mimeType?: string, target?: string): Cypress.Chainable<void>,
+
+			/**
+			 * Reset the admin theming entirely.
+			 * **Warning**: Using this function will reset the previous session
+			 */
+			resetAdminTheming(): Cypress.Chainable<void>,
+
+			/**
+			 * Reset the user theming settings.
+			 * If provided, will clear session and login as the given user.
+			 * **Warning**:  Providing a user will reset the previous session.
+			 */
+			resetUserTheming(user?: User): Cypress.Chainable<void>,
 		}
 	}
 }
@@ -89,7 +105,7 @@ Cypress.Commands.add('uploadFile', (user, fixture = 'image.jpg', mimeType = 'ima
 /**
  * Reset the admin theming entirely
  */
- Cypress.Commands.add('resetTheming', () => {
+ Cypress.Commands.add('resetAdminTheming', () => {
 	const admin = new User('admin', 'admin')
 
 	cy.clearCookies()
@@ -111,3 +127,33 @@ Cypress.Commands.add('uploadFile', (user, fixture = 'image.jpg', mimeType = 'ima
 	// Clear admin session
 	cy.clearCookies()
 })
+
+/**
+ * Reset the current or provided user theming settings
+ * It does not reset the theme config as it is enforced in the
+ * server config for cypress testing.
+ */
+Cypress.Commands.add('resetUserTheming', (user?: User) => {
+	if (user) {
+		cy.clearCookies()
+		cy.login(user)
+	}
+
+	// Reset background config
+	cy.request('/csrftoken').then(({ body }) => {
+		const requestToken = body.token
+
+		cy.request({
+			method: 'POST',
+			url: '/apps/theming/background/default',
+			headers: {
+				'requesttoken': requestToken,
+			},
+		})
+	})
+
+	if (user) {
+		// Clear current session
+		cy.clearCookies()
+	}
+})