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

allow using the full height of the header for the logo #4633

Closed
wants to merge 1 commit into from

Conversation

icewind1991
Copy link
Member

Before

image

After

image

The stock logo will still have the same size since that one is also limited by width

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label May 1, 2017
@icewind1991 icewind1991 added this to the Nextcloud 12.0 milestone May 1, 2017
@mention-bot
Copy link

@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @juliushaertl, @skjnldsv and @jancborchardt to be potential reviewers.

@MorrisJobke
Copy link
Member

cc @nextcloud/designers

@MorrisJobke
Copy link
Member

The Nextcloud logo stays the same in size and the branded one looks like this (before and after)

bildschirmfoto 2017-05-01 um 18 20 35
bildschirmfoto 2017-05-01 um 18 24 11

I think it's okay to reduce the padding. It allows more flexibility what to show there.

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.

I’m a bit skeptical here … the logo seems to be so small only because you have a bunch of padding, no? In @MorrisJobke’s test it becomes apparent that using the full height can result in a very ugly header, and we don’t want people to accidentally have ugly themes.

@MorrisJobke
Copy link
Member

I’m a bit skeptical here … the logo seems to be so small only because you have a bunch of padding, no? In @MorrisJobke’s test it becomes apparent that using the full height can result in a very ugly header, and we don’t want people to accidentally have ugly themes.

I also thought about this. So we just say: remove the padding from the logo and use the CSS padding. Fine by me too.

Let's close this here then.

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

Successfully merging this pull request may close these issues.

4 participants