Simplified Nextcloud logo icon #2

[pixelipo]

Replaces pull request #5288

This is part of the SVG optimization task #5157

Nextcloud logo code has been simplified, but at the same time it's more human-readable.

I've changed the size to more traditional values (256*128). Circles are a bit fatter than they used to be (+0.5px at 128px width). If that is an issue, I can change their width to 21.5px.

logo-icon.svg is deprecated since new logo.svg is already much smaller in weight.

I've combined .logo and .logo-icon - this is not strictly needed, I can revert if @nextcloud/designers disagree. It requires adding logo class to tags that have logo-icon class in apps - mainly where pages are shared publicly, e.g. Calendar.

PNG images and favicons were not edited.

Signed-off-by: Marin Treselj marin@pixelipo.com

[pixelipo]

As mentioned in #5288 if we want to limit the width of the logo to 175px, why not just make the container smaller? No need to check mime-type here

[juliusknorr]

The problem is that themed logos should use all of the space that is available, while the default logo should just have its default size.

[pixelipo]

Ok, I see your point. I can make adjustments.
But in that case, can I ask why is the width set to 252px? parent div, .header, is 300px wide (probably in order to fit into smallest supported mobile width), while the form elements (inputs and submit) are 269px wide. Seems arbitrary...

[juliusknorr]

Maybe @jancborchardt has an idea about that?

[jancborchardt]

Yes, it is completely arbitrary and has some historical reasons I don't remember. The basic points are of course that:

• It should work well on mobile
• All the elements (input field and button) on log in should have the same width.
• Error messages need to be checked

If you can make that work, we can fix it all to 270 px or even 300px if that looks good.

[jancborchardt]

But maybe best let's merge this first, and do thaf in a separate pull request? Your call @pixelipo :)

[pixelipo]

@jancborchardt yes, that should be a separate PR (and a separate issue for discussion).

Should I revert this or do we merge as is?

[jancborchardt]

@pixelipo yes, that part should be reverted ideally because otherwise master is broken. :)

[codecov]

Codecov Report

Merging #5407 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #5407      +/-   ##
============================================
- Coverage     53.98%   53.98%   -0.01%     
  Complexity    22466    22466              
============================================
  Files          1390     1390              
  Lines         85980    85980              
  Branches       1329     1329              
============================================
- Hits          46419    46416       -3     
- Misses        39561    39564       +3

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/templates/public.php	0% <0%> (ø)	0 <0> (ø)	⬇️
core/templates/layout.user.php	0% <0%> (ø)	0 <0> (ø)	⬇️
apps/comments/lib/EventHandler.php	79.16% <0%> (-8.34%)	7% <0%> (ø)
apps/files_external/lib/Lib/Storage/SMB.php	50.94% <0%> (-0.38%)	116% <0%> (ø)
lib/private/Server.php	93.36% <0%> (-0.15%)	120% <0%> (ø)
lib/private/Security/CertificateManager.php	91.83% <0%> (+1.02%)	39% <0%> (ø)	⬇️

[pixelipo]

Should be ready to review/merge, @jancborchardt @juliushaertl @MorrisJobke

[juliusknorr]

I've rebased this to master. Works as expected. 👍

[jancborchardt]

Lools good to me now - @juliushaertl @MorrisJobke?

[juliusknorr]

@jancborchardt Yes, already approved by me 😉 CI failing is unrelated.