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

Remove gradient header for themed instances #19669

Closed
wants to merge 2 commits into from

Conversation

earboxer
Copy link

Close #19040 as suggested by @juliushaertl

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews design Design, UI, UX, etc. feature: theming labels Feb 27, 2020
@rullzer rullzer added this to the Nextcloud 19 milestone Feb 28, 2020
Copy link
Member

@juliushaertl juliushaertl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan was to keep the gradient for the default Nextcloud blue, but do not apply it if a custom theming color is set.

Adding a color check like

@if ($color-primary == #0082C9) or ($has-custom-background == true) {
to the main background fade mixin in
background-image: linear-gradient(40deg, $color-primary 0%, lighten($color-primary, 20%) 100%);
should do the trick.

@juliushaertl juliushaertl added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 2, 2020
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gradients are so 1990 ...

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @juliushaertl, the gradient should only be removed when using a custom theming color.

And gradients being so 1990 makes them in fashion again now ;)
image

@bpcurse
Copy link

bpcurse commented Mar 13, 2020

Milestone Nextcloud 19

Will this be backported to 18?

@earboxer
Copy link
Author

I added the color check as described by @juliushaertl, but I haven't been able to test it...

What do we have to run on our servers to compile the SCSS? (and it's aware of the user-defined color at compile time?)

@rullzer
Copy link
Member

rullzer commented Apr 2, 2020

@juliushaertl @jancborchardt good to go now?

@@ -50,6 +50,9 @@
z-index: 2000;
height: $header-height;
background-color: var(--color-primary);
@if ($color-primary == #0082C9) {
background-image: linear-gradient(40deg, $color-primary 0%, lighten($color-primary, 20%) 100%);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the default background image. That change should go to theming.scss. Maybe you can try something like this, to make sure that the gradient is properly set otherwise:

diff --git a/apps/theming/css/theming.scss b/apps/theming/css/theming.scss
index 0e9b922b27..c2a58dcc3d 100644
--- a/apps/theming/css/theming.scss
+++ b/apps/theming/css/theming.scss
@@ -8,7 +8,11 @@
 }
 
 @mixin faded-background {
-       background-image: linear-gradient(40deg, $color-primary 0%, lighten($color-primary, 20%) 100%);
+       @if ($color-primary == #0082C9) {
+               background-image: linear-gradient(40deg, $color-primary 0%, lighten($color-primary, 20%) 100%);
+       } @else {
+               background-color: $color-primary;
+       }
 }
 
 @mixin faded-background-image {
@@ -23,6 +27,10 @@
                @include faded-background;
                background-size: contain;
        }
+
+       @if ($color-primary != #0082C9) and ($has-custom-background == false) {
+               background-image: none;
+       }
 }

@juliushaertl
Copy link
Member

What do we have to run on our servers to compile the SCSS? (and it's aware of the user-defined color at compile time?)

SCSS files should get reloaded automatically when debug mode is enabled. Otherwise you can always run occ maintenance:repair to remove all cached CSS.

@rullzer rullzer mentioned this pull request Apr 4, 2020
80 tasks
@kesselb
Copy link
Contributor

kesselb commented Apr 8, 2020

Conflicts after #20343.

This was referenced Apr 9, 2020
@rullzer rullzer mentioned this pull request Apr 23, 2020
11 tasks
@jancborchardt jancborchardt changed the title Fix header gradient (by removing it) Remove gradient header for themed instances Apr 23, 2020
@rullzer rullzer modified the milestones: Nextcloud 19, Nextcloud 20 Apr 30, 2020
@juliushaertl juliushaertl self-assigned this May 29, 2020
@juliushaertl
Copy link
Member

Thanks for getting this started. I've moved my suggestion to #21198 and did a bit of code cleanup in the same run as well so we can get this merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress design Design, UI, UX, etc. feature: theming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make gradient optional
8 participants