Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix background theming deletion and cypress admin theming tests #35728

Merged
merged 3 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/theming/lib/Controller/ThemingController.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public function updateStylesheet($setting, $value) {
}
break;
case 'disable-user-theming':
if ($value !== "yes" && $value !== "no") {
if ($value !== 'yes' && $value !== 'no') {
$error = $this->l10n->t('Disable-user-theming should be true or false');
}
break;
Expand Down
7 changes: 4 additions & 3 deletions apps/theming/lib/ImageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ public function getImageUrlAbsolute(string $key): string {
* @throws NotPermittedException
*/
public function getImage(string $key, bool $useSvg = true): ISimpleFile {
$logo = $this->config->getAppValue('theming', $key . 'Mime', '');
$mime = $this->config->getAppValue('theming', $key . 'Mime', '');
$folder = $this->getRootFolder()->getFolder('images');

if ($logo === '' || !$folder->fileExists($key)) {
if ($mime === '' || !$folder->fileExists($key)) {
throw new NotFoundException();
}

Expand All @@ -127,7 +127,8 @@ public function getImage(string $key, bool $useSvg = true): ISimpleFile {

public function hasImage(string $key): bool {
$mimeSetting = $this->config->getAppValue('theming', $key . 'Mime', '');
return $mimeSetting !== '';
// Removing the background defines its mime as 'backgroundColor'
return $mimeSetting !== '' && $mimeSetting !== 'backgroundColor';
}

/**
Expand Down
19 changes: 15 additions & 4 deletions apps/theming/lib/Themes/CommonThemeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected function generateGlobalBackgroundVariables(): array {
foreach (ImageManager::SUPPORTED_IMAGE_KEYS as $image) {
if ($this->imageManager->hasImage($image)) {
$imageUrl = $this->imageManager->getImageUrl($image);
// --image-background is overridden by user theming
// --image-background is overridden by user theming if logged in
$variables["--image-$image"] = "url('" . $imageUrl . "')";
}
}
Expand Down Expand Up @@ -127,15 +127,17 @@ protected function generateUserBackgroundVariables(): array {
if ($user !== null
&& !$this->themingDefaults->isUserThemingDisabled()
&& $this->appManager->isEnabledForUser(Application::APP_ID)) {
$adminBackgroundDeleted = $this->config->getAppValue(Application::APP_ID, 'backgroundMime', '') === 'backgroundColor';
$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);
$isPrimaryBright = $this->util->invertTextColor($this->themingDefaults->getColorPrimary());

// The user removed the background
if ($backgroundImage === BackgroundService::BACKGROUND_DISABLED) {
return [
'--image-background' => 'no',
'--color-background-plain' => $this->primaryColor,
// Might be defined already by admin theming, needs to be overridden
'--image-background' => 'none',
'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
// If no background image is set, we need to check against the shown primary colour
'--background-image-invert-if-bright' => $isPrimaryBright ? 'invert(100%)' : 'no',
];
Expand All @@ -150,6 +152,15 @@ protected function generateUserBackgroundVariables(): array {
];
}

// The user is using the default background and admin removed the background image
if ($backgroundImage === BackgroundService::BACKGROUND_DEFAULT && $adminBackgroundDeleted) {
return [
// --image-background is not defined in this case
'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
'--background-image-invert-if-bright' => $isPrimaryBright ? 'invert(100%)' : 'no',
];
}

// The user picked a shipped background
if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$backgroundImage])) {
return [
Expand Down
2 changes: 1 addition & 1 deletion apps/theming/lib/Themes/DefaultTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function __construct(Util $util,
$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary();
$this->primaryColor = $this->themingDefaults->getColorPrimary();

// Override default defaultPrimaryColor if set to improve accessibility
// Override primary colors (if set) to improve accessibility
if ($this->primaryColor === BackgroundService::DEFAULT_COLOR) {
$this->primaryColor = BackgroundService::DEFAULT_ACCESSIBLE_COLOR;
}
Expand Down
4 changes: 3 additions & 1 deletion apps/theming/lib/ThemingDefaults.php
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,9 @@ public function getColorPrimary(): string {
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 BackgroundService::DEFAULT_COLOR;
}

return $color;
}

Expand Down Expand Up @@ -487,6 +488,7 @@ public function undo($setting): string {
case 'background':
case 'favicon':
$this->imageManager->delete($setting);
$this->config->deleteAppValue('theming', $setting . 'Mime');
break;
}

Expand Down
4 changes: 2 additions & 2 deletions apps/theming/src/AdminTheming.vue
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,12 @@ export default {
/* This is basically https://github.com/nextcloud/server/blob/master/core/css/guest.css
But without the user variables. That way the admin can preview the render as guest*/
/* As guest, there is no user color color-background-plain */
background-color: var(--color-primary-element-default, #0082c9);
background-color: var(--color-primary-element-default);
/* As guest, there is no user background (--image-background)
1. Empty background if defined
2. Else default background
3. Finally default gradient (should not happened, the background is always defined anyway) */
background-image: var(--image-background-plain, var(--image-background-default, linear-gradient(40deg, #0082c9 0%, #30b6ff 100%)));
background-image: var(--image-background-plain, var(--image-background-default));

&-logo {
width: 20%;
Expand Down
2 changes: 1 addition & 1 deletion core/css/apps.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion core/css/apps.css.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions core/css/apps.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ html {
body {
// color-background-plain should always be defined. It is the primary user colour
background-color: var(--color-background-plain, var(--color-main-background));
// image-background-plain means the admin has disabled the background image
background-image: var(--image-background, var(--image-background-default));
// user background, or plain colour and finally default admin background
background-image: var(--image-background, var(--image-background-plain, var(--image-background-default)));
background-size: cover;
background-position: center;
position: fixed;
Expand Down
2 changes: 1 addition & 1 deletion core/css/server.css

Large diffs are not rendered by default.

Loading