Skip to content

Fixes the logo height - #11384

Merged
rullzer merged 1 commit into
masterfrom
11372/fix/logo-height
Sep 27, 2018
Merged

Fixes the logo height#11384
rullzer merged 1 commit into
masterfrom
11372/fix/logo-height

Conversation

@weeman1337

@weeman1337weeman1337 commented Sep 26, 2018•
edited
Loading

Copy link
Copy Markdown
Member

before
image

after
image

Closes#11372

@weeman1337weeman1337 added design Design, UI, UX, etc. 3. to review Waiting for reviews papercut Annoying recurring UX issue with possibly simple fix. labels Sep 26, 2018
@weeman1337weeman1337 mentioned this pull request Sep 26, 2018
Merged
skjnldsv
skjnldsv reviewed Sep 26, 2018
View reviewed changes
Comment threadcore/css/header.scss Outdated
background-position: center;
width: 62px;
height: 100%;
height: calc(100% - 2px);

@skjnldsvskjnldsvSep 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm cinfused, where does those 2px come from? 🤔 😁

@weeman1337weeman1337Sep 26, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@skjnldsv it has top: 1px;. When there should be the same at the bottom a height of 100 % minus two pixels would to that.

@skjnldsvskjnldsvSep 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we set a top at 0?
I remember something about this, but I can't recall 😁

@skjnldsvskjnldsvSep 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Found the guilty man: #10853
Damn you @skjnldsv!!!!

@weeman1337weeman1337Sep 26, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

ha ha :) I tried it without the tiny spacing. In my opinion it looks better like this...

@skjnldsvskjnldsvSep 26, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you could add a bottom: 1px? so we understand better?
I'm fine with that, but i'm afraid we'll forget about this like I did above 😝

@weeman1337weeman1337Sep 26, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@skjnldsv done ✓

@go2sh

go2sh commented Sep 26, 2018

Copy link
Copy Markdown
Contributor

I tested it also. 👍

@skjnldsv

skjnldsv commented Sep 26, 2018

Copy link
Copy Markdown
Member

@go2sh you should be able to add your review on the top right ;)

go2sh
go2sh approved these changes Sep 26, 2018
View reviewed changes

@go2shgo2sh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested and works. :)

@weeman1337@rullzer
Fixes the logo height
c7714b4
Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
@rullzer
rullzerforce-pushed the 11372/fix/logo-height branch from 46bd4c4 to c7714b4CompareSeptember 27, 2018 10:47
@rullzer
rullzer merged commit fbe270d into masterSep 27, 2018
@rullzer
rullzer deleted the 11372/fix/logo-height branch September 27, 2018 13:41
@MorrisJobke

MorrisJobke commented Oct 1, 2018

Copy link
Copy Markdown
Member

@weeman1337 Once the backport PR is open you could remove the backport-request label. Because we go regularly through the closed PRs that still have the label to backport those. This then makes it easier for us to spot the not yet backported. Just a little hint for the future and to explain how we work. :)

@MorrisJobkeMorrisJobke added this to the Nextcloud 15 milestone Oct 1, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzerrullzerrullzer approved these changes
@go2shgo2shgo2sh approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr
@danxuliudanxuliuAwaiting requested review from danxuliu

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsdesignDesign, UI, UX, etc.papercutAnnoying recurring UX issue with possibly simple fix.

Projects

None yet

Milestone

Nextcloud 15

Development

Successfully merging this pull request may close these issues.

5 participants

@weeman1337@go2sh@skjnldsv@MorrisJobke@rullzer