Skip to content

Conversation

@Warkot
Copy link
Contributor

@Warkot Warkot commented Sep 20, 2016

Footer has to be ready to properly render these changes: Wikia/app#11396.

@Wikia/x-wing

@hakubo
Copy link
Contributor

hakubo commented Sep 20, 2016

Should we update reference page?

display: flex;
justify-content: center;
margin: 0;
padding: 30px 0 25px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it look how it should with the current logos?

@Warkot
Copy link
Contributor Author

Warkot commented Sep 20, 2016

Yes, we should update the reference page. We should also support the case where there's no wds-is-en or wds-is-international class added. I'll cover these.


&__header-logo {
height: 45px;
width: 185px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why 185 x 45? SVG has width="184" height="44".

Copy link
Contributor

Choose a reason for hiding this comment

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

I like round numbers :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're perfectly rounded to full pixel. ;)

<svg width="254" height="26" viewBox="0 0 254 26" xmlns="http://www.w3.org/2000/svg">
<defs>
<linearGradient x1="0%" y1="100%" x2="63.848%" y2="32.54%" id="logo-fandom-powered-by-wikia-gradient">
<linearGradient x1="0%" y1="100%" x2="63.848%" y2="32.54%" id="a">
Copy link

Choose a reason for hiding this comment

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

This is not a good change, I think we should manually revert / fix all those ids to be unique.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was probably caused by svgo run without config. I'll revert that.

Copy link
Contributor Author

@Warkot Warkot Sep 21, 2016

Choose a reason for hiding this comment

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

Fixed that. However, the new SVG logo-fandom-powered-by-wikia-two-lines.svg had just generic IDs like path-1. The SVGs merged from master also have generic IDs like a.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, was it a single comment or a review? It didn't disappear after the changes. 😞

@kvas-damian
Copy link
Contributor

In this PR we changes logo spacing for international version - see VRT. Was it intended?

@Warkot
Copy link
Contributor Author

Warkot commented Sep 21, 2016

@hakubo could you answer @kvas-damian's comment #68 (comment)?

@hakubo
Copy link
Contributor

hakubo commented Sep 21, 2016

@kvas-damian not intended. but this way we minimise the work needed to be done on Oct 4th.
as we won't have to add any classes to it.

@hakubo
Copy link
Contributor

hakubo commented Sep 21, 2016

confirmed with Derek, it's fine to change these paddings

@kvas-damian
Copy link
Contributor

Please update VRT!

@Warkot Warkot merged commit 14a0d03 into master Sep 22, 2016
@Warkot Warkot deleted the XW-2076 branch September 22, 2016 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants