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

Updates logo scss to regard default values and updates favicon upload check #10963

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

weeman1337
Copy link
Member

@weeman1337 weeman1337 commented Sep 2, 2018

Logo
With the change from here: e8938df the scss variables from theming were overwritten by the value in the variables.scss.

This fix adds support for default values to the logo and background scss.

Favicon
SVG favicons are not working if they're SVG and imagemagick isn't present. There is already a notice on the theming page, but the user is still able to upload a SVG favicon. I changed this so the upload returns "unsupported image type" for SVG favicons without imagemagick.

Closes #10957

@weeman1337 weeman1337 changed the title Updates logo scss to regard default values Updates logo scss to regard default values and updates favicon upload check Sep 2, 2018
@weeman1337
Copy link
Member Author

For me the failed tests look unrelated. But someone should double check.

@weeman1337 weeman1337 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 2, 2018
@weeman1337 weeman1337 added this to the Nextcloud 14 milestone Sep 2, 2018
@skjnldsv
Copy link
Member

skjnldsv commented Sep 2, 2018

I restarted the tests. It looks unrelated yes :)

@rullzer rullzer modified the milestones: Nextcloud 14, Nextcloud 15 Sep 3, 2018
@juliusknorr
Copy link
Member

juliusknorr commented Sep 4, 2018

@rullzer Any chance we can still include 84fa208 into the 14 release? Because otherwise the theming of logos is broken 🙈 Other option would be to revert e8938df and include both fixes in 14.0.1

@rullzer
Copy link
Member

rullzer commented Sep 4, 2018

@juliushaertl yes sure if it is a regression lets do it. Changes are minor it seems.

@juliusknorr
Copy link
Member

All right, created a backport PR for the first commit, since that is the important one to fix the regression: #11048

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
@rullzer rullzer force-pushed the fix/10957/logo-scss branch from bfc638b to 38ea5d1 Compare September 5, 2018 18:50
@rullzer
Copy link
Member

rullzer commented Sep 5, 2018

Rebased

@rullzer rullzer merged commit 50a6a7c into master Sep 5, 2018
@rullzer rullzer deleted the fix/10957/logo-scss branch September 5, 2018 21:15
@nickvergessen
Copy link
Member

@weeman1337 do you know, should this go into 14 too?

@weeman1337
Copy link
Member Author

@nickvergessen I think #11055 should go to.
@juliushaertl closed it. Maybe it's solved by another PR?

@MorrisJobke
Copy link
Member

@nickvergessen I think #11055 should go to.
@juliushaertl closed it. Maybe it's solved by another PR?

@juliushaertl Any comments why you closed it?

@MorrisJobke
Copy link
Member

@juliushaertl 🏓 ;)

@juliusknorr
Copy link
Member

I closed it since @rullzer already opened a correct backport request for that PR in #11052

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

14 RC2: problems with logo and favicon
6 participants