Skip to content

Commit

Permalink
Fix background removal not applying to user default theming
Browse files Browse the repository at this point in the history
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
  • Loading branch information
skjnldsv committed Dec 11, 2022
1 parent e92725a commit 89911b4
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 24 deletions.
14 changes: 12 additions & 2 deletions apps/theming/lib/Themes/CommonThemeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ 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);
Expand All @@ -144,15 +145,24 @@ protected function generateUserBackgroundVariables(): array {
$cacheBuster = substr(sha1($user->getUID() . '_' . $currentVersion), 0, 8);
return [
'--image-background' => "url('" . $this->urlGenerator->linkToRouteAbsolute('theming.userTheme.getBackground') . "?v=$cacheBuster')",
'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
'--color-background-plain' => $this->primaryColor,
];
}

// The user is using the default background and admin removed the background image
if ($backgroundImage === BackgroundService::BACKGROUND_DEFAULT && $adminBackgroundDeleted) {
return [
'--color-background-plain' => $this->primaryColor,
// Default background is not inverted
'--background-image-invert-if-bright' => 'no',
];
}

// The user picked a shipped background
if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$backgroundImage])) {
return [
'--image-background' => "url('" . $this->urlGenerator->linkTo(Application::APP_ID, "img/background/$backgroundImage") . "')",
'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
'--color-background-plain' => $this->primaryColor,
'--background-image-invert-if-bright' => BackgroundService::SHIPPED_BACKGROUNDS[$backgroundImage]['theming'] ?? null === BackgroundService::THEMING_MODE_DARK ? 'invert(100%)' : 'no',
];
}
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 @@ -299,12 +299,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-default, #0082c9);
background-color: var(--color-primary-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 apps/theming/src/components/BackgroundSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ export default {

&__default {
background-color: var(--color-primary-default);
background-image: var(--image-background-default);
background-image: var(--image-background-plain, var(--image-background-default));
}

&__filepicker, &__default, &__color {
Expand Down
2 changes: 1 addition & 1 deletion core/css/apps.css

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 @@ -46,8 +46,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));
// color-background-plain should always be defined. It is the primary user colour
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/guest.css
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ body {
2. Empty background if enabled ('yes' is used, that way the variable is _defined_)
3. Else default background
4. Finally default gradient (should not happened, the background is always defined anyway) */
background-image: var(--image-background, var(--image-background-plain, var(--image-background-default, linear-gradient(40deg, #0082c9 0%, #30b6ff 100%))));
background-image: var(--image-background, var(--image-background-plain, var(--image-background-default)));
background-attachment: fixed;
min-height: 100%; /* fix sticky footer */
height: auto;
Expand Down
2 changes: 1 addition & 1 deletion core/css/server.css

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

58 changes: 54 additions & 4 deletions cypress/e2e/theming/admin-settings.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('Change the primary color and reset it', function() {
})
})

describe.only('Remove the default background and restore it', function() {
describe('Remove the default background and restore it', function() {
before(function() {
// Just in case previous test failed
cy.resetAdminTheming()
Expand All @@ -108,11 +108,10 @@ describe.only('Remove the default background and restore it', function() {
cy.get('[data-admin-theming-setting-file-remove]').click()

cy.wait('@removeBackground')
cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
cy.waitUntil(() => cy.window().then((win) => {
const currentBackgroundDefault = getComputedStyle(win.document.body).getPropertyValue('--image-background-default')
const backgroundPlain = getComputedStyle(win.document.body).getPropertyValue('--image-background-plain')
return !currentBackgroundDefault.includes(defaultBackground)
&& backgroundPlain !== ''
return backgroundPlain !== ''
}))
})

Expand Down Expand Up @@ -407,3 +406,54 @@ describe('The user default background settings reflect the admin theming setting
cy.waitUntil(() => validateUserThemingDefaultCss(selectedColor, '/apps/theming/image/background?v='))
})
})

describe('The user default background settings reflect the admin theming settings with background removed', function() {
before(function() {
// Just in case previous test failed
cy.resetAdminTheming()
cy.login(admin)
})

after(function() {
cy.resetAdminTheming()
})

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')
cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
})

it('Login page should match admin theming settings', function() {
cy.logout()
cy.visit('/')

cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
})

it('Login as user', function() {
cy.createRandomUser().then((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('Default user background settings should match admin theming settings', function() {
cy.get('[data-user-theming-background-default]').should('be.visible')
cy.get('[data-user-theming-background-default]').should('have.class', 'background--active')

cy.waitUntil(() => validateUserThemingDefaultCss(defaultPrimary, null))
})
})
10 changes: 6 additions & 4 deletions cypress/e2e/theming/themingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ export const validateBodyThemingCss = function(expectedColor = '#0082c9', expect
const guestBackgroundColor = getComputedStyle(win.document.body).backgroundColor
const guestBackgroundImage = getComputedStyle(win.document.body).backgroundImage

const isValidBackgroundImage = expectedBackground === null
const isValidBackgroundColor = colord(guestBackgroundColor).isEqual(expectedColor)
const isValidBackgroundImage = !expectedBackground
? guestBackgroundImage === 'none'
: guestBackgroundImage.includes(expectedBackground)

return colord(guestBackgroundColor).isEqual(expectedColor)
&& isValidBackgroundImage
console.debug({ guestBackgroundColor, guestBackgroundImage, expectedColor, expectedBackground, isValidBackgroundColor, isValidBackgroundImage })

return isValidBackgroundColor && isValidBackgroundImage
})
}

Expand All @@ -59,7 +61,7 @@ export const validateUserThemingDefaultCss = function(expectedColor = '#0082c9',
const defaultOptionBorderColor = getComputedStyle(defaultSelectButton).borderColor
const colorPickerOptionColor = getComputedStyle(customColorSelectButton).backgroundColor

const isValidBackgroundImage = expectedBackground === null
const isValidBackgroundImage = !expectedBackground
? defaultOptionBackground === 'none'
: defaultOptionBackground.includes(expectedBackground)

Expand Down
4 changes: 2 additions & 2 deletions dist/theming-admin-theming.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/theming-admin-theming.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/theming-personal-theming.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/theming-personal-theming.js.map

Large diffs are not rendered by default.

0 comments on commit 89911b4

Please sign in to comment.