Merge pull request #34696 from nextcloud/fix/custom-color

Allow to remove the background and select a custom colour + cypress

.drone.yml

@@ -1715,36 +1715,6 @@ trigger:
     - pull_request
     - push
 
----
-kind: pipeline
-name: acceptance-app-theming
-
-steps:
-- name: submodules
-  image: ghcr.io/nextcloud/continuous-integration-alpine-git:latest
-  commands:
-    - git submodule update --init
-- name: acceptance-app-theming
-  image: ghcr.io/nextcloud/continuous-integration-acceptance-php7.4:latest
-  commands:
-    - tests/acceptance/run-local.sh --timeout-multiplier 10 --nextcloud-server-domain acceptance-app-theming --selenium-server selenium:4444 allow-git-repository-modifications features/app-theming.feature
-
-services:
-- name: selenium
-  image: ghcr.io/nextcloud/continuous-integration-selenium:3.141.59
-  environment:
-    # Reduce default log level for Selenium server (INFO) as it is too
-    # verbose.
-    JAVA_OPTS: -Dselenium.LOGGER.level=WARNING
-
-trigger:
-  branch:
-    - master
-    - stable*
-  event:
-    - pull_request
-    - push
-
 ---
 kind: pipeline
 name: acceptance-header

.github/workflows/cypress.yml

@@ -0,0 +1,98 @@
+name: Cypress
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+      - stable*
+
+env:
+  APP_NAME: viewer
+  BRANCH: ${{ github.base_ref }}
+  TESTING: true
+
+jobs:
+  init:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout server
+        uses: actions/checkout@v3
+
+      - name: Read package.json node and npm engines version
+        uses: skjnldsv/read-package-engines-version-actions@v1.2
+        id: versions
+        with:
+          fallbackNode: "^12"
+          fallbackNpm: "^6"
+
+      - name: Set up node ${{ steps.versions.outputs.nodeVersion }}
+        uses: actions/setup-node@v3
+        with:
+          cache: 'npm'
+          node-version: ${{ steps.versions.outputs.nodeVersion }}
+
+      - name: Set up npm ${{ steps.versions.outputs.npmVersion }}
+        run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}"
+
+      - name: Install dependencies & build app
+        run: |
+          npm ci
+          TESTING=true npm run build --if-present
+
+      - name: Save context
+        uses: actions/cache@v3
+        with:
+          key: cypress-context-${{ github.run_id }}
+          path: /home/runner/work/server
+
+  cypress:
+    runs-on: ubuntu-latest
+    needs: init
+
+    strategy:
+      fail-fast: false
+      matrix:
+        # run multiple copies of the current job in parallel
+        containers: [1]
+
+    name: runner ${{ matrix.containers }}
+
+    steps:
+      - name: Restore context
+        uses: actions/cache@v3
+        with:
+          key: cypress-context-${{ github.run_id }}
+          path: /home/runner/work/server
+
+      - name: Run E2E cypress tests
+        uses: cypress-io/github-action@v4
+        with:
+          record: true
+          parallel: true
+          # cypress env
+          ci-build-id: ${{ github.sha }}-${{ github.run_number }}
+          tag: ${{ github.event_name }}
+        env:
+          # Needs to be prefixed with CYPRESS_
+          CYPRESS_BRANCH: ${{ env.BRANCH }}
+          CYPRESS_GH: true
+          # https://github.com/cypress-io/github-action/issues/124
+          COMMIT_INFO_MESSAGE: ${{ github.event.pull_request.title }}
+          # Needed for some specific code workarounds
+          TESTING: true
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
+
+  summary:
+    runs-on: ubuntu-latest
+    needs: [init, cypress]
+
+    if: always()
+
+    name: cypress-summary
+
+    steps:
+      - name: Summary status
+        run: if ${{ needs.init.result != 'success' || ( needs.cypress.result != 'success' && needs.cypress.result != 'skipped' ) }}; then exit 1; fi

.gitignore

@@ -163,3 +163,7 @@ composer.phar
 
 ./.htaccess
 core/js/mimetypelist.js
+
+# Tests - cypress
+cypress/snapshots
+cypress/videos

apps/theming/appinfo/routes.php

@@ -88,6 +88,11 @@
 			'url' => '/background/{type}',
 			'verb' => 'POST',
 		],
+		[
+			'name' => 'userTheme#deleteBackground',
+			'url' => '/background/custom',
+			'verb' => 'DELETE',
+		],
 	],
 	'ocs' => [
 		[

apps/theming/css/default.css

@@ -54,9 +54,6 @@
   --background-invert-if-dark: no;
   --background-invert-if-bright: invert(100%);
   --background-image-invert-if-bright: no;
-  --image-background: url('/core/img/app-background.jpg');
-  --image-background-default: url('/core/img/app-background.jpg');
-  --color-background-plain: #0082c9;
   --primary-invert-if-bright: no;
   --color-primary: #006aa3;
   --color-primary-default: #0082c9;
@@ -75,4 +72,6 @@
   --color-primary-element-light-hover: #dbe5ea;
   --color-primary-element-text-dark: #ededed;
   --gradient-primary-background: linear-gradient(40deg, var(--color-primary) 0%, var(--color-primary-hover) 100%);
+  --image-background-default: url('/apps/theming/img/background/kamil-porembinski-clouds.jpg');
+  --color-background-plain: #0082c9;
 }

apps/theming/lib/Command/UpdateConfig.php

@@ -36,10 +36,6 @@ class UpdateConfig extends Command {
 		'name', 'url', 'imprintUrl', 'privacyUrl', 'slogan', 'color', 'disable-user-theming'
 	];
 
-	public const SUPPORTED_IMAGE_KEYS = [
-		'background', 'logo', 'favicon', 'logoheader'
-	];
-
 	private $themingDefaults;
 	private $imageManager;
 	private $config;
@@ -87,14 +83,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 				$value = $this->config->getAppValue('theming', $key, '');
 				$output->writeln('- ' . $key . ': ' . $value . '');
 			}
-			foreach (self::SUPPORTED_IMAGE_KEYS as $key) {
+			foreach (ImageManager::SUPPORTED_IMAGE_KEYS as $key) {
 				$value = $this->config->getAppValue('theming', $key . 'Mime', '');
 				$output->writeln('- ' . $key . ': ' . $value . '');
 			}
 			return 0;
 		}
 
-		if (!in_array($key, self::SUPPORTED_KEYS, true) && !in_array($key, self::SUPPORTED_IMAGE_KEYS, true)) {
+		if (!in_array($key, self::SUPPORTED_KEYS, true) && !in_array($key, ImageManager::SUPPORTED_IMAGE_KEYS, true)) {
 			$output->writeln('<error>Invalid config key provided</error>');
 			return 1;
 		}
@@ -115,7 +111,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 0;
 		}
 
-		if (in_array($key, self::SUPPORTED_IMAGE_KEYS, true)) {
+		if (in_array($key, ImageManager::SUPPORTED_IMAGE_KEYS, true)) {
 			if (strpos($value, '/') !== 0) {
 				$output->writeln('<error>The image file needs to be provided as an absolute path: ' . $value . '.</error>');
 				return 1;

apps/theming/lib/Controller/UserThemeController.php

@@ -155,25 +155,43 @@ public function getBackground(): Http\Response {
 	/**
 	 * @NoAdminRequired
 	 */
-	public function setBackground(string $type = 'default', string $value = ''): JSONResponse {
+	public function deleteBackground(): JSONResponse {
+		$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
+		$this->backgroundService->deleteBackgroundImage();
+		return new JSONResponse([
+			'backgroundImage' => null,
+			'backgroundColor' => $this->themingDefaults->getColorPrimary(),
+			'version' => $currentVersion,
+		]);
+	}
+
+	/**
+	 * @NoAdminRequired
+	 */
+	public function setBackground(string $type = BackgroundService::BACKGROUND_DEFAULT, string $value = '', string $color = null): JSONResponse {
 		$currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
 
+		// Set color if provided
+		if ($color) {
+			$this->backgroundService->setColorBackground($color);
+		}
+
+		// Set background image if provided
 		try {
 			switch ($type) {
-				case 'shipped':
+				case BackgroundService::BACKGROUND_SHIPPED:
 					$this->backgroundService->setShippedBackground($value);
 					break;
-				case 'custom':
+				case BackgroundService::BACKGROUND_CUSTOM:
 					$this->backgroundService->setFileBackground($value);
 					break;
-				case 'color':
-					$this->backgroundService->setColorBackground($value);
-					break;
-				case 'default':
+				case BackgroundService::BACKGROUND_DEFAULT:
 					$this->backgroundService->setDefaultBackground();
 					break;
 				default:
-					return new JSONResponse(['error' => 'Invalid type provided'], Http::STATUS_BAD_REQUEST);
+					if (!$color) {
+						return new JSONResponse(['error' => 'Invalid type provided'], Http::STATUS_BAD_REQUEST);
+					}
 			}
 		} catch (\InvalidArgumentException $e) {
 			return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
@@ -185,8 +203,8 @@ public function setBackground(string $type = 'default', string $value = ''): JSO
 		$this->config->setUserValue($this->userId, Application::APP_ID, 'userCacheBuster', (string)$currentVersion);
 
 		return new JSONResponse([
-			'type' => $type,
-			'value' => $value,
+			'backgroundImage' => $this->config->getUserValue($this->userId, Application::APP_ID, 'background_image', BackgroundService::BACKGROUND_DEFAULT),
+			'backgroundColor' => $this->themingDefaults->getColorPrimary(),
 			'version' => $currentVersion,
 		]);
 	}

apps/theming/lib/ImageManager.php

@@ -33,6 +33,8 @@
  */
 namespace OCA\Theming;
 
+use OCA\Theming\AppInfo\Application;
+use OCA\Theming\Service\BackgroundService;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
@@ -45,7 +47,7 @@
 use OCP\IURLGenerator;
 
 class ImageManager {
-	public const SupportedImageKeys = ['background', 'logo', 'logoheader', 'favicon'];
+	public const SUPPORTED_IMAGE_KEYS = ['background', 'logo', 'logoheader', 'favicon'];
 
 	/** @var IConfig */
 	private $config;
@@ -74,8 +76,14 @@ public function __construct(IConfig $config,
 		$this->appData = $appData;
 	}
 
-	public function getImageUrl(string $key, bool $useSvg = true): string {
-		$cacheBusterCounter = $this->config->getAppValue('theming', 'cachebuster', '0');
+	/**
+	 * Get a globally defined image (admin theming settings)
+	 *
+	 * @param string $key the image key
+	 * @return string the image url
+	 */
+	public function getImageUrl(string $key): string {
+		$cacheBusterCounter = $this->config->getAppValue(Application::APP_ID, 'cachebuster', '0');
 		if ($this->hasImage($key)) {
 			return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter;
 		}
@@ -86,13 +94,16 @@ public function getImageUrl(string $key, bool $useSvg = true): string {
 			case 'favicon':
 				return $this->urlGenerator->imagePath('core', 'logo/logo.png') . '?v=' . $cacheBusterCounter;
 			case 'background':
-				return $this->urlGenerator->imagePath('core', 'background.png') . '?v=' . $cacheBusterCounter;
+				return $this->urlGenerator->linkTo(Application::APP_ID, 'img/background/' . BackgroundService::DEFAULT_BACKGROUND_IMAGE);
 		}
 		return '';
 	}
 
-	public function getImageUrlAbsolute(string $key, bool $useSvg = true): string {
-		return $this->urlGenerator->getAbsoluteURL($this->getImageUrl($key, $useSvg));
+	/**
+	 * Get the absolute url. See getImageUrl
+	 */
+	public function getImageUrlAbsolute(string $key): string {
+		return $this->urlGenerator->getAbsoluteURL($this->getImageUrl($key));
 	}
 
 	/**
@@ -136,6 +147,20 @@ public function hasImage(string $key): bool {
 		return $mimeSetting !== '';
 	}
 
+	/**
+	 * @return array<string, array{mime: string, url: string}>
+	 */
+	public function getCustomImages(): array {
+		$images = [];
+		foreach (self::SUPPORTED_IMAGE_KEYS as $key) {
+			$images[$key] = [
+				'mime' => $this->config->getAppValue('theming', $key . 'Mime', ''),
+				'url' => $this->getImageUrl($key),
+			];
+		}
+		return $images;
+	}
+
 	/**
 	 * Get folder for current theming files
 	 *

apps/theming/lib/Listener/BeforeTemplateRenderedListener.php

@@ -84,16 +84,32 @@ public function handle(Event $event): void {
 		if (!empty($user)) {
 			$userId = $user->getUID();
 
+			/** User background */
 			$this->initialState->provideInitialState(
-				'background',
-				$this->config->getUserValue($userId, Application::APP_ID, 'background', 'default'),
+				'backgroundImage',
+				$this->config->getUserValue($userId, Application::APP_ID, 'background_image', BackgroundService::BACKGROUND_DEFAULT),
 			);
 
+			/** User color */
+			$this->initialState->provideInitialState(
+				'backgroundColor',
+				$this->config->getUserValue($userId, Application::APP_ID, 'background_color', BackgroundService::DEFAULT_COLOR),
+			);
+
+			/** 
+			 * Admin background. `backgroundColor` if disabled,
+			 * mime type if defined and empty by default
+			 */
 			$this->initialState->provideInitialState(
 				'themingDefaultBackground',
 				 $this->config->getAppValue('theming', 'backgroundMime', ''),
 			);
+			$this->initialState->provideInitialState(
+				'defaultShippedBackground',
+				 BackgroundService::DEFAULT_BACKGROUND_IMAGE,
+			);
 
+			/** List of all shipped backgrounds */
 			$this->initialState->provideInitialState(
 				'shippedBackgrounds',
 				 BackgroundService::SHIPPED_BACKGROUNDS,